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

fish_clipboard_paste: trim indentation when pasting multiple lines #7662

Merged
merged 2 commits into from Feb 13, 2021

Conversation

krobelus
Copy link
Member

Description

When pasting a multiline command with indented blocks, extra indentation from
spaces, or tabs, is generally undesirable. Discard it unless the cursor is
inside quotes.


This is probably not always exactly what you wants, since sometimes you might paste multiline tokens that are not commands.
But it's surely better in the majority of all cases.

I also created this PR because I wasn't sure about __fish_commandline_is_singlequoted.
Is it okay to rename it? It seems that some people are using it already, but not many.
Maybe it should be fish_commandline_is_quoted single for the old behavior, and fish_commandline_is_quoted single double for "single or double", but the "print" variant is best for fish_clipboard_paste, so we don't need to call it twice.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@faho
Copy link
Member

faho commented Jan 29, 2021

I also created this PR because I wasn't sure about __fish_commandline_is_singlequoted.
Is it okay to rename it? It seems that some people are using it already, but not many.

It's a dunderscored function, which we technically don't give guarantees for (but tbh we have sooo many of these, some we even recommend in the docs, that that's not a fantastic policy in practice - maybe we should undunder some?), but it's also a very specialized thing.

I say go for it, if you think anyone is using it (my gut says probably not, but I have been wrong before) add a compatibility shim - see __fish_git_prompt.


I have two questions here:

  1. How well does this handle arbitrary pastes? Escaped things, quoted strings spanning multiple lines and such?
  2. Wouldn't this be better solved by stripping it when doing the commandline insert? fish_clipboard_paste only does one commandline -i, so it should be possible to handle that as one bit and ignore any superfluous indentation. Or maybe actually run the indentation code on it? (I can see, of course, how this is quite a bit simpler, so it might be okay to add this and eventually replace it)

@krobelus
Copy link
Member Author

There are some occurrences on GitHub. It's probably still acceptable to rename it.

How well does this handle arbitrary pastes?

Hmm, sadly it doesn't handle quoted multiline strings. Pasting something like

echo '
    indented

drops the indent. This is fixable.

  1. Wouldn't this be better solved by stripping it when doing the commandline insert?

Could be. Adding this to commandline -i can work better, because we could always detect if we are inside quotes.
I can't think of cases we would break but commandline -i ought to be reliable.

We could also use the logic from __fish_commandline_is_singlequoted on $data to avoid trimming the quoted string example.
I'll think about it.

A similar issue is when I edit a multiline command with Alt-E and run fish_indent on it in the editor. When going back to the commandline, the indentation is doubled. Anyway, that's easily avoidable.

@krobelus krobelus force-pushed the trim-indentation-when-pasting branch from e67b06a to e334826 Compare February 8, 2021 07:14
@krobelus
Copy link
Member Author

krobelus commented Feb 8, 2021

It might be a good idea to un-dunder functions that are likely to be copied by users (for example because they sample prompts). However, that will just add to the functions we'd want to un-dunder/document/keep supporting. So I'm not sure.

I'm still not entirely happy with the name, maybe it should stay dundered.

I'm printing a deprecation message plus stack trace when the old function is loaded, but keep the behavior. The same should be usable for other deprecated wrappers. I still need to test how this will look like when called from bindings. Okay that got lost in the rebase, but it wouldn't look nice, so I'm dropping that.

@krobelus krobelus force-pushed the trim-indentation-when-pasting branch from e334826 to 6271a31 Compare February 13, 2021 07:13
@krobelus krobelus added this to the fish 3.2.0 milestone Feb 13, 2021
When pasting a multiline command with indented blocks, extra indentation
from spaces, or tabs, is generally undesirable, because fish already indents
pipes and blocks. Discard the indentation unless the cursor or the pasted
part is inside quotes.

Users who copied fish_clipboard_paste need to update it because
__fish_commandline_is_singlequoted had an API change and was renamed.
…quoted

It could be really annoying if Ctrl-V stopped working when switching between
fish versions, for example when using different machines.
@krobelus krobelus force-pushed the trim-indentation-when-pasting branch from 6271a31 to 114aa99 Compare February 13, 2021 07:55
@krobelus krobelus merged commit e16a1d7 into fish-shell:master Feb 13, 2021
@wd wd mentioned this pull request Mar 5, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants