Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make pasting literals easier #967

Closed
mrshu opened this issue Aug 21, 2013 · 45 comments
Closed

Make pasting literals easier #967

mrshu opened this issue Aug 21, 2013 · 45 comments
Assignees
Milestone

Comments

@mrshu
Copy link
Contributor

mrshu commented Aug 21, 2013

I've just ran into this:

$ wget https://addons.mozilla.org/firefox/downloads/latest/161972/addon-161972-latest.xml?src=search
fish: No matches for wildcard 'https://addons.mozilla.org/firefox/downloads/latest/161972/addon-161972-latest.xml?src=search'.
wget https://addons.mozilla.org/firefox/downloads/latest/161972/addon-161972-latest.xml?src=search
     ^

It's obvious that '?' causes this but why is the caret at the beginning of the argument? Maybe it would be worth it finding the wildcard and putting the caret below that?

@ridiculousfish
Copy link
Member

Good suggestion. This also trips up a lot of people who are accustomed to bash's behavior of passing literal wildcards through when they would not expand to anything. Maybe we can also add URL detection or something.

@mrshu
Copy link
Contributor Author

mrshu commented Aug 21, 2013

@ridiculousfish thanks for looking at this.

If you were open to some solution which would be easy to implement I'd be all for trying to do that.

@ahti
Copy link
Contributor

ahti commented Jun 19, 2014

Although treating wildcards differently when they look like urls is introducing inconsistency, I still think it would be a reasonable thing to do, because it is, imho, what the user expects.

When a user enters an url into their terminal, I think it is reasonable to assume that in 99.9% of cases, they don't want it to behave like a wildcard, but just like a plain old url.

@maxnordlund
Copy link
Contributor

I've recently encountered this problem as well. I've been trying to come up with a good solution that fits into the design philosophy of fish and after reading through some of the linked issues this seams indeed very hard.

But then I thought, why not allow a user defined function to modify pasted text before it is inserted? Basically something that is called when the user pastes something, and it's output would be what's inserted. This would make any escaping clearly visible, and allow the user to apply any escaping she might desire.

To make this easier fish should probably ship a couple of example implementations, but probably not actually enable any by default, since that might be too surprising for the user.


If for example the clipboard contains the string https://www.example.com/wiki/index.php?page=About this could be one way of doing it:

~$ wget <C^p>
~$ wget https://www.example.com/wiki/index.php\?page=About

Another way would be to always wrap any pasted text in quotes, and might be good default behaviour:

~$ wget <C^p>
~$ wget "https://www.example.com/wiki/index.php?page=About"

@zanchey
Copy link
Member

zanchey commented Jan 30, 2015

One of the problems is working out a way of detecting pasted text. Should fish treat all keypresses within a certain threshold as pasted? What about when your connection is briefly interrupted and your keystrokes queue up in mosh - as that will probably appear the same?

@ridiculousfish
Copy link
Member

We could always enhance the terminal itself. It's sort of ridiculous how primitive the communication channel is from terminal to shell - we get signals like SIGWINCH, and not much else. But there have been various efforts to improve it - Final Term, etc.

@maxnordlund
Copy link
Contributor

I found a page about bracketed paste that seams to solve this nicely for most terminals, but for anything else I guess you just have to count characters entered during a short amount of time.

@zanchey
Copy link
Member

zanchey commented Jan 30, 2015

Ha. That's rad.

@ridiculousfish
Copy link
Member

Wow, I've never heard of bracketed paste before! Nifty.

@zanchey
Copy link
Member

zanchey commented Nov 13, 2015

There's some notes on http://thejh.net/misc/website-terminal-copy-paste about implementing bracketed paste safely - importantly, escaping the "unbracket" escape sequence.

@ridiculousfish ridiculousfish self-assigned this Jan 13, 2016
@faho faho changed the title Wildcard problem with wget Wildcard problem with URLs (wget) Feb 21, 2016
@faho faho mentioned this issue Jul 2, 2016
@faho
Copy link
Member

faho commented Oct 19, 2016

Does this still seem like a good thing to do? Personally, I find the \cv binding for fish_clipboard_paste much more convenient than \cV (my terms paste) anyway.

@floam
Copy link
Member

floam commented Oct 19, 2016

Well, I want it and prefer to use my terminals paste a lot, and I think a lot of users can be helped.

Couldn't we implement bracketed paste entirely with a few clever bind commands?

@faho
Copy link
Member

faho commented Oct 19, 2016

Couldn't we implement bracketed paste entirely with a few clever bind commands?

I don't think so. We have no way of saying that anything should be ignored, currently. Of course if bind took a real glob, like bind "\e[*\e[" (where "\e[" stands for the bracketed paste sequences) and the matched part was given as argument to the called function it would be possible, but AFAIK that's currently not the way it works. And adding that seems like a lot more work. Though that might be useful for numeric arguments for vi-mode.

@faho
Copy link
Member

faho commented Oct 22, 2016

Well, turns out implementing the basics is rather easy:

    # This sequence starts paste-mode
    bind -M default -m paste \e\[200~ ''
    # In paste-mode, everything self-inserts except for the sequence to get out of it
    bind -M paste "" self-insert
    # TODO: This should return to the previous mode (vi)
    bind -M paste -m default \e\[201~ force-repaint
    # Without this, a \r will overwrite the other text, rendering it invisible - which makes the exercise kinda pointless.
    bind -M paste \r "commandline -i \n"
    # Enable bracketed paste mode
    printf "\e[?2004h"

There's a few open questions here:

  • This is still rather slow, probably because we're still highlighting on every character. One of the advantages of bracketed paste is that we don't need to do that.
  • Does this need to be conditionalized? Is that starting sequence (the printf stuff) either supported or ignored everywhere so we can just print it? The bindings aren't really harmful - you're not gonna get that sequence in a non-supporting terminal.
  • How do we adjust it so things are auto-escaped? Do we wrap everything in "''" and bind "'" to "'"? Plus of course doubling "". That means you could just paste one token.

Also:

escaping the "unbracket" escape sequence.

This is something the terminal needs to do.

Konsole does not do this, xterm does.

@floam
Copy link
Member

floam commented Oct 22, 2016

Neato!

@floam
Copy link
Member

floam commented Oct 22, 2016

Probably needs to disable bracketed paste mode before executing a program and at exit.

@maxnordlund
Copy link
Contributor

maxnordlund commented Oct 22, 2016

I would like to see the following behaviour:

If I have an empty command line, and paste, it should not be escaped but also not executed even if the pasted text contains newlines. For that we need an explicit confirmation from the user, e.g. pressing enter.

If I instead have something in my command line, e.g. wget, then it should escape the pasted text/wrap it in quotes, but like above not run the command even if it had newlines. One neat thing about wrapping it in quotes is that the syntax highlighting should be easier.


And as @floam said, it needs to be scoped to fish. So turn it on when fish gets control over stdin, and turn it off before it releases said control. This would be running other programs, backgrounded (if that's a thing), and before exiting.

@faho faho self-assigned this Oct 22, 2016
@floam
Copy link
Member

floam commented Oct 22, 2016

That makes sense to me, to quote it if and only if there is already a command on the line.

@maxnordlund
Copy link
Contributor

I can confirm it working iTerm/ViM on Mac OS Sierra at least.

@floam
Copy link
Member

floam commented Feb 3, 2017

I just installed emacs 25.1 from Homebrew - OS X ships with 22.something.

Works fine for me in Terminal.app. Maybe the guy had a weird key binding.

@floam floam changed the title support bracketed-paste mode to avoid problems with pasted text containing wildcards support bracketed-paste mode Feb 3, 2017
@leavengood
Copy link

It sounds like @faho is now working on this either way, but I just wanted to mention that it seems supporting bracketed paste in fish really has no downside.

My understanding is that adding the escape codes that "bracket" the paste is up to the terminal emulator. So you would only see them in supported terminals when you send the escape code which "turns on" bracketed paste mode (as indicated on https://cirw.in/blog/bracketed-paste.)

If the terminal doesn't support bracketed paste you will never see those "bracket" escape codes. If you DO see those brackets then you can alter the behavior of fish (such as to treat newlines as part of a multiline edit so as not to execute the pasted commands immediately, or whatever else makes sense.)

If a user doesn't like bracketed paste mode they can disable it in their terminal emulator.

So I really see no good reason why fish should not support it. Of course someone has to do the work to add support for it 😄

@faho
Copy link
Member

faho commented Feb 25, 2017

If the terminal doesn't support bracketed paste you will never see those "bracket" escape codes.

Well.... You won't see the "start paste" or "stop paste" codes, but you will see the "hey, fish supports bracketed paste" code. Or rather, the way this works is that the client program, in this case fish, needs to send "please bracket pastes!", and then the terminal reacts in one of three different ways:

  • It swallows the sequence and enables bracketed paste

  • It swallows the sequence and doesn't enable bracketed paste

  • It doesn't get the sequence and shows it, at least partially

The third one is the issue, and unfortunately I don't know of a way to detect that. Note that the latest emacs version seems to think this can't happen, but then again the latest emacs version is the first to support bracketed paste as a host (so it seems that they think they were the last ever to support it). I don't know if older versions swallowed the sequence like they should (at least when run outside of a terminal, e.g. as a gtk program).

@maxnordlund
Copy link
Contributor

The least supporting terminals I can think of is the Linux terminal* and xterm, but I think the behave OK.

*The one you drop into when to don't have x.org, aka real terminal

@faho
Copy link
Member

faho commented Feb 25, 2017

The least supporting terminals I can think of is the Linux terminal* and xterm

Xterm was the first to support this, and it actually has quite a few features. Whenever you need an example of a "bad" terminal, see emacs' "ansi-term".

@maxnordlund
Copy link
Contributor

Auch, yeah but I don't really count that as a real terminal. It even says it's support is spotty (or so I recall).

If you run Linux, could you test this in the terminal?

@faho
Copy link
Member

faho commented Feb 25, 2017

Linux VT swallows the sequence, I don't think pasting is even possible in that context.

I'm gearing up towards a PR now, should be "real soon"(tm), unless I'm interrupted or find another issue.

faho added a commit to faho/fish-shell that referenced this issue Feb 25, 2017
This is a terminal feature where pastes will be "bracketed" in
\e\[200~ and \e\[201~.

It is more of a "security" measure (since particularly copying from a
browser can copy text different from what the user sees, which might
be malicious) than a performance optimization.

Fixes fish-shell#967.
@krader1961
Copy link
Contributor

The following is in response to a question posed in PR #3871.

Personally I find @GReagle's argument in issue #967 more compelling than doing magical escaping some times but not others. I guess an argument could be made to only magically escape question-marks since they seem to be, for all intents and purposes, the only character that causes problems. But what do you do if the user has typed wget ' before pasting? And there are probably similar scenarios that complicate doing the right thing. And why just strings that look like URLs? And how exactly would we do so in a robust manner? If we decide to do something about pasting strings with question-marks which don't represent a wildcard character I think we should just bite the bullet and behave like bash/zsh and pass the question-mark through literally without raising an error. We would still treat asterisks like we do today. That has its own problems if the question-mark, when treated as a wildcard, just happens to match a file. Unlikely but not impossible. This is why I am uneasy about adding another special-case rule. At the end of the day I vote for consistency. Which is to say we don't second-guess the user and require them to quote the pasted text if it has a question-mark or asterisk they don't want treated as a wildcard.

@maxnordlund
Copy link
Contributor

I think the point is that you paste rather then manually type in the input. When I do the latter it's easy to escape, but if I paste something it gets harder. Sure I can add quotes, but if there's already quotes/hash/dollar sign and whatnot in the string it quickly becomes tedious.

As I said in #967 (comment) I think quoting should only happen in the argument position, so pasting a command works like now. However if already have wget/git remote add/… then it quotes the pasted text as it's given as an argument. That would be the least surprising behaviour I think.

The alternative to sometimes ignore wildcards is worse IMO because it depends on the hidden state of the filesystem. Take the example of pasting markdown the **main** point …, that is recursively expanded and any main files get caught. Another example is git's commitish, which uses a lot of special symbols, git diff master@{yesterday}~4..HEAD^!.

One could argue that those cases a few and far behind, but it would still be very surprising behaviour.


A side note, all of ?/&/%/# regularly shows up in URLs, and have special meaning in fish.

@faho
Copy link
Member

faho commented Mar 2, 2017

The alternative to sometimes ignore wildcards is worse IMO because it depends on the hidden state of the filesystem.

Of course, and I don't think @krader1961 was seriously suggesting that.

A side note, all of ?/&/%/# regularly shows up in URLs, and have special meaning in fish.

"%" and "#" are not an issue there because they only have special meaning at the beginning of a token, i.e. echo a#b prints "a#b".

For some reason "&" is special everywhere - echo a&echo foo will run "echo a" in the background and "echo foo". Arguably, that's wrong, and I'll post an issue about it.

So for URLs that leaves "?", which we've talked about removing.

Take the example of pasting markdown the main point …, that is recursively expanded and any main files get caught.

Yes, they do. The question here is what you want to happen when you paste **main**. Either you want to have the glob, or, when you think of it as markdown, you'll want to have '**main**' appear instead. Since fish doesn't know that you are currently looking at markdown, it has to guess. If it guesses "this is supposed to be code" and it was supposed to be markdown, then you'll need to go in and escape it - or you could have started quoting before the paste. If it guesses "this is supposed to be literal" and it was supposed to be code, then you'll need to go in and unescape it. The issue here is that you couldn't have started quoting before.


One possible behavior I've thought about is this:

  • By default, paste as-is - this makes pasting code easy

  • If, when the paste starts, the current token has an unbalanced ', escape any pasted ' - this allows pasting non-code

Obviously there are a few variants of this:

  • Only check if the token ends with a ', under the assumption that the user is going to deal with other things later

  • Also use " and either also only escape " (thereby letting $ through) or escape both " and $ (which would make this just cosmetically different from ')

The question is if this is understandable enough. I'll try to code something up to play around with. I can tell you already that \' is going to be an issue.

@maxnordlund
Copy link
Contributor

I guess the only really nice solution is dependent on the type of argument for the current command. When the docopt branch lands and we can specify the argument type <URL>/<FILES>, this could be made smarter. Well, at least if it's possible to annotate those as to be escaped.

But that's not viable now, and I like the escape-on-quote mechanic. If I already mark the stuff I'm about to paste as a string, it should be quite intuitive when it escapes quotes in that string. I also like the "/$ escape combo, coming from double quote languages I get bitten by pasting stuff with dollar signs, even though I didn't intend it to do that.

@faho
Copy link
Member

faho commented Mar 2, 2017

I guess the only really nice solution is dependent on the type of argument for the current command. When the docopt branch lands and we can specify the argument type /, this could be made smarter.

I'd argue you don't want that, for a multitude of reasons:

  • The rule is much harder - it's "when fish thinks this is something that shouldn't be expanded"

  • Sometimes you want to actually have an argument expand into a URL

  • The docopt system won't have 100% coverage of every tool ever

In short: Something that can correctly predict the future 50% of the time is really really impressive, but also infuriating.


I've coded up a rough version of this both with bracketed-paste and fish_clipboard_paste.

Here's the latter, since that's easier to test:

function fish_clipboard_paste
    set -l data
    if type -q pbpaste
        set data (pbpaste)
    else if type -q xsel
        # Return if `xsel` failed.
        # That way any xsel error is printed (to show e.g. a non-functioning X connection),
        # but we don't print the redundant (and overly verbose for this) commandline error.
        # Also require non-empty contents to not clear the buffer.
        if not set data (xsel --clipboard)
            return 1
        end
    end
    # If the current token has an unmatched single-quote,
    # escape all single-quotes (and backslashes) in the paste,
    # in order to turn it into a single literal token.
    #
    # This eases pasting non-code (e.g. markdown or git commitishes).
    # TODO: This does not handle
    # single-quotes quoted in double-quotes.

    # Remove escaped single-quotes first so that detecting unbalanced ones is easier.
    if commandline -ct | string replace -r "(\\\\)*\\\'" "" \
    | string match -qr "^([^']*'[^']*'|[^']*)*'('[^']*'|[^']*)*\$"
        set data (string replace -ra "(['\\\])" '\\\\\\\$1' -- $data)
    end
    if test -n "$data"
        commandline -i -- "$data"
    end
end

So far, it seems to feel okay, though I've mostly been pasting weird stuff like just ' or \\\'

@faho faho changed the title support bracketed-paste mode Make pasting literals easier Mar 2, 2017
@faho
Copy link
Member

faho commented Mar 2, 2017

I've now retitled this to be closer to the original issue - there's nothing we can or should make special about URLs, but we can fix pasting.

I'll change my PR to not close this when merging.

Just to be clear though: Bracketed paste is a necessity to fix this for terminal-side paste (i.e. not using fish_clipboard_paste) because without it we cannot distinguish between normal user input and pasting.

faho added a commit that referenced this issue Mar 5, 2017
This is a terminal feature where pastes will be "bracketed" in
\e\[200~ and \e\[201~.

It is more of a "security" measure (since particularly copying from a
browser can copy text different from what the user sees, which might
be malicious) than a performance optimization.

Work towards #967.
faho added a commit to faho/fish-shell that referenced this issue Mar 9, 2017
This is to make pasting literals easier.

When a user pastes something, we normally take it as-is.

The exception is when a single-quote is open, e.g. the current token
is

    foo'bar

When something is pasted here, we escape single-quotes (`'`) and
backslashes (`\\`), so typing a `'` after it will turn it into a
literal token.

Fixes fish-shell#967.
@faho faho closed this as completed in 99e87dd Mar 16, 2017
@faho faho modified the milestones: 2.6.0, next-major Mar 16, 2017
develop7 pushed a commit to develop7/fish-shell that referenced this issue Apr 17, 2017
This is a terminal feature where pastes will be "bracketed" in
\e\[200~ and \e\[201~.

It is more of a "security" measure (since particularly copying from a
browser can copy text different from what the user sees, which might
be malicious) than a performance optimization.

Work towards fish-shell#967.
develop7 pushed a commit to develop7/fish-shell that referenced this issue Apr 17, 2017
This is to make pasting literals easier.

When a user pastes something, we normally take it as-is.

The exception is when a single-quote is open, e.g. the current token
is

    foo'bar

When something is pasted here, we escape single-quotes (`'`) and
backslashes (`\\`), so typing a `'` after it will turn it into a
literal token.

Fixes fish-shell#967.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
10 participants