[PATCH] bracketed-paste: change quoting style

[PATCH] bracketed-paste: change quoting style

[Daniel Shahaf]

Make bracketed-paste use ''-quoting instead of \-quoting (when NUMERIC
is nonzero).

It's a bikeshed, of course, but I think '' is more readable.

diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index 502e41e..5f2a580 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -779,7 +779,7 @@ bracketedpaste(char **args)
 	int n;
 	ZLE_STRING_T wpaste;
 	wpaste = stringaszleline((zmult == 1) ? pbuf :
-	    quotestring(pbuf, NULL, QT_BACKSLASH), 0, &n, NULL, NULL);
+	    quotestring(pbuf, NULL, QT_SINGLE_OPTIONAL), 0, &n, NULL, NULL);
 	cuttext(wpaste, n, CUT_REPLACE);
 	if (!(zmod.flags & MOD_VIBUF)) {
 	    kct = -1;

Re: [PATCH] bracketed-paste: change quoting style

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 12:09 AM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Make bracketed-paste use ''-quoting instead of \-quoting (when NUMERIC
> is nonzero).
>
> It's a bikeshed, of course, but I think '' is more readable.
>
> diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
> index 502e41e..5f2a580 100644
> --- a/Src/Zle/zle_misc.c
> +++ b/Src/Zle/zle_misc.c
> @@ -779,7 +779,7 @@ bracketedpaste(char **args)
>         int n;
>         ZLE_STRING_T wpaste;
>         wpaste = stringaszleline((zmult == 1) ? pbuf :
> -           quotestring(pbuf, NULL, QT_BACKSLASH), 0, &n, NULL, NULL);
> +           quotestring(pbuf, NULL, QT_SINGLE_OPTIONAL), 0, &n, NULL, NULL);
>         cuttext(wpaste, n, CUT_REPLACE);
>         if (!(zmod.flags & MOD_VIBUF)) {
>             kct = -1;

If we go here, we may as well allow the numeric to pick which quoting
style to use. :)

On a related note, someone on irc was confused about why
url-quote-magic stopped working in git, and it's of course because
bracketed pasting causes its self-insert hook to never run. We may
want to provide an alternative function to help with this case. (eg,
wrap bracketed-paste and quote if == http* or so.)

-- 
Mikael Magnusson

Re: [PATCH] bracketed-paste: change quoting style

[Bart Schaefer]

On Aug 12,  3:08am, Mikael Magnusson wrote:
}
} On a related note, someone on irc was confused about why
} url-quote-magic stopped working in git, and it's of course because
} bracketed pasting causes its self-insert hook to never run.

Hrm.  I'm tempted simply to suggest that bracketed-paste should get
out of the way if there are no newlines in the pasted text, but
there may be other bindings that shouldn't run.

On a different somewhat related note, bracketedpaste() lacks the tests
for invalid multibyte characters that selfinsert() performs.

} We may want to provide an alternative function to help with this case.
} (eg, wrap bracketed-paste and quote if == http* or so.)

This gets ugly if the URLs are somewhere in the middle of the pasted
text.

Re: [PATCH] bracketed-paste: change quoting style

[Mikael Magnusson]

On Wed, Aug 12, 2015 at 4:52 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 12,  3:08am, Mikael Magnusson wrote:
> }
> } On a related note, someone on irc was confused about why
> } url-quote-magic stopped working in git, and it's of course because
> } bracketed pasting causes its self-insert hook to never run.
>
> Hrm.  I'm tempted simply to suggest that bracketed-paste should get
> out of the way if there are no newlines in the pasted text, but
> there may be other bindings that shouldn't run.

It's also nice for avoiding various other problems, like tabs.

I feel like the widget is already getting quite DWIMmy, and what "we"
mean might not be what everyone else means. (I'm not even convinced we
all want the things it's already doing).

I would also argue that intentionally putting text in the paste buffer
to invoke widgets by pasting it is somewhat perverse, and if you want
to do it, you get to change the default configuration to do it. It
shouldn't depend on something mostly orthogonal like if there is a
newline.

I thought earlier we may even want to add a
bracketed-paste-raw/simple/literal function that just does what
bracketed-paste does in the case of being passed a parameter, but also
inserts it on the command line for you, and nothing more. It gives you
an easier knob to disable the DWIM stuff than having to write a custom
widget.

> On a different somewhat related note, bracketedpaste() lacks the tests
> for invalid multibyte characters that selfinsert() performs.
>
> } We may want to provide an alternative function to help with this case.
> } (eg, wrap bracketed-paste and quote if == http* or so.)
>
> This gets ugly if the URLs are somewhere in the middle of the pasted
> text.

Well, it's probably only worth handling if the url is the only word.

-- 
Mikael Magnusson

Re: [PATCH] bracketed-paste: change quoting style

[Bart Schaefer]

On Aug 12,  5:13am, Mikael Magnusson wrote:
} Subject: Re: [PATCH] bracketed-paste: change quoting style
}
} On Wed, Aug 12, 2015 at 4:52 AM, Bart Schaefer
} <schaefer@brasslantern.com> wrote:
} > I'm tempted simply to suggest that bracketed-paste should get
} > out of the way if there are no newlines in the pasted text, but
} > there may be other bindings that shouldn't run.
} 
} It's also nice for avoiding various other problems, like tabs.

How about this?

    bracketed-paste-magic() {
      local PASTED
      zle .bracketed-paste PASTED
      zle -U - $PASTED
      while [[ -n $PASTED ]] && zle .read-command
      do
	PASTED=${PASTED#$KEYS}
	case $REPLY in
	(self-insert*) zle $REPLY;;
	(*) zle self-insert-unmeta;;
	esac
      done
    }
    zle -N bracketed-paste bracketed-paste-magic

I actually wonder if implementing the above loop in C might not be
preferable to flatly calling doinsert().

} I feel like the widget is already getting quite DWIMmy, and what "we"
} mean might not be what everyone else means. (I'm not even convinced we
} all want the things it's already doing).

Yes, the above breaks the (so far undocumented?) vi-mode cutbuffer stuff,
doesn't handle overwriting the region or setting undo points, probably
breaks somehow with multibyte characters, etc.

} I would also argue that intentionally putting text in the paste buffer
} to invoke widgets by pasting it is somewhat perverse

I'm not sure what you're talking about here.  I'd put text in the paste
buffer in order to paste it.  For pasting to result in a widget being
invoked is no different than any other interaction with the editor and
amounts to an implemention detail of the real desire, which is to paste
the text without something unexpected happening.

Of course if what you are expecting is for it to behave as if you typed
each character individually, bracketed-paste is going to confuse you.  

} and if you want to do it, you get to change the default configuration
} to do it. It shouldn't depend on something mostly orthogonal like if
} there is a newline.

"If there is a newline" is shorthand for "if any action other than a
self-insert would occur".

} I thought earlier we may even want to add a
} bracketed-paste-raw/simple/literal function that just does what
} bracketed-paste does in the case of being passed a parameter, but also
} inserts it on the command line for you, and nothing more.

Yes, I could see the utility of this.

Re: [PATCH] bracketed-paste: change quoting style

[Oliver Kiddle]

Daniel Shahaf wrote:
> Make bracketed-paste use ''-quoting instead of \-quoting (when NUMERIC
> is nonzero).
> 
> It's a bikeshed, of course, but I think '' is more readable.

I'd agree that it is less ugly but there was some rationale behind the
choice of \-quoting. It is easier to remove unwanted characters. So
if the paste inserts text\  you can press backspace twice to get rid
of the trailing space. With 'text ', you have to do more. It is also
more likely to give you something useful if you are nesting quotes.
Finally, it is what completion uses if there is no existing quoting so
is consistent with that.

I can't say I care a great deal. If you do, Mikael's suggestion of
making it depend on the numeric argument is a good idea.

What I was hoping to get at with the vim-style `[ and `] marks was a way
to retrospectively quote the pasted text. Doing things in that order
seems more helpful and more flexible.

Oliver

Re: [PATCH] bracketed-paste: change quoting style

[Bart Schaefer]

On Aug 13,  4:49pm, Oliver Kiddle wrote:
}
} I'd agree that it is less ugly but there was some rationale behind the
} choice of \-quoting. It is easier to remove unwanted characters.

It's easier to remove the unwanted characters if there isn't very much
text being pasted or if all the unwanted characters are at the end ...
but if there are a bunch of backslashes scattered throughout a large
paste it gets ugly.

} What I was hoping to get at with the vim-style `[ and `] marks was a way
} to retrospectively quote the pasted text. Doing things in that order
} seems more helpful and more flexible.

Yes, invoking quote-region after the paste allows you to first clean up
the tail end before adding the quotes, so you get the best of both.

Re: [PATCH] bracketed-paste: change quoting style

[Daniel Shahaf]

Bart Schaefer wrote on Thu, Aug 13, 2015 at 09:20:48 -0700:
> On Aug 13,  4:49pm, Oliver Kiddle wrote:
> }
> } I'd agree that it is less ugly but there was some rationale behind the
> } choice of \-quoting. It is easier to remove unwanted characters.
> 
> It's easier to remove the unwanted characters if there isn't very much
> text being pasted or if all the unwanted characters are at the end ...
> but if there are a bunch of backslashes scattered throughout a large
> paste it gets ugly.

Exactly.

> } What I was hoping to get at with the vim-style `[ and `] marks was a way
> } to retrospectively quote the pasted text. Doing things in that order
> } seems more helpful and more flexible.
> 
> Yes, invoking quote-region after the paste allows you to first clean up
> the tail end before adding the quotes, so you get the best of both.

I agree that separating the "paste it" and "quote it" to separate steps
would be good, but it doesn't seem to work in current git: invoking
quote-region quotes the entire command line, not only the pasted part.
Does it depend on an uncommitted patch or something?

If the sequence <bracketed-paste><quote-region> worked, then I'd suggest
to make bracketed-paste not check NUMERIC at all (and never quote the
pasted text), so we could assign NUMERIC a different meaning in the
future, if we wanted to.  (I suppose this echoes Mikael's remarks about
a minimal bracketed-paste-raw.)

Cheers,

Daniel

Re: [PATCH] bracketed-paste: change quoting style

[Bart Schaefer]

On Aug 13, 11:19pm, Daniel Shahaf wrote:
} 
} I agree that separating the "paste it" and "quote it" to separate steps
} would be good, but it doesn't seem to work in current git: invoking
} quote-region quotes the entire command line, not only the pasted part.
} Does it depend on an uncommitted patch or something?

No, but you have to set the mark before pasting; the action of pasting
does not make the region active (though it replaces the active region
if there is one).

Re: [PATCH] bracketed-paste: change quoting style

[Daniel Shahaf]

Bart Schaefer wrote on Thu, Aug 13, 2015 at 16:57:09 -0700:
> On Aug 13, 11:19pm, Daniel Shahaf wrote:
> } 
> } I agree that separating the "paste it" and "quote it" to separate steps
> } would be good, but it doesn't seem to work in current git: invoking
> } quote-region quotes the entire command line, not only the pasted part.
> } Does it depend on an uncommitted patch or something?
> 
> No, but you have to set the mark before pasting; the action of pasting
> does not make the region active (though it replaces the active region
> if there is one).

I see; so the workflow is <set-mark-command><paste><quote-region>.  This is
nice.  However, I will keep using <NUMERIC><paste>, because it's fewer
keypresses.

As to changing the default quoting style, I went ahead and pushed it.  (Using
single quotes is consistent with quote-region and quote-line.)  I saw the
discussion about which values of NUMERIC should cause quoting (36193) but
I think it is orthogonal.

As Oliver said — we can make further changes later if needed.

Cheers,

Daniel

P.S.  Someone pointed out on IRC that 'typeset -p IFS' prints a literal
newline character.  Perhaps it should use $''-quoting to escape the
newline.