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

Pasting multiple lines into commit summary field executes commands #3151

Closed
OliverJAsh opened this issue Dec 8, 2023 · 9 comments · Fixed by #4234
Closed

Pasting multiple lines into commit summary field executes commands #3151

OliverJAsh opened this issue Dec 8, 2023 · 9 comments · Fixed by #4234
Labels
bug Something isn't working

Comments

@OliverJAsh
Copy link

Describe the bug
When pasting multiple lines into commit summary field, any characters on lines after the first line will be executed as commands.

To Reproduce
Steps to reproduce the behavior:

  1. Stage a change.
  2. Open the commit form (c).
  3. Paste this into the commit summary field:
    B
    o
    

Expected behavior
Only the first line is used. Subsequent lines are discarded.

OR

Subsequent lines are pasted into the commit description field.

Screenshots

Screen.Recording.2023-12-08.at.10.41.31.mov

Version info:
lazygit --version: 0.40.2
git --version: 2.42.0

Additional context
This happens to me frequently when I paste the contents of the "copy commit message" shortcut. In this example it's just opening a file (o), but in some cases it can cause much more harm.

@OliverJAsh OliverJAsh added the bug Something isn't working label Dec 8, 2023
@stefanhaller
Copy link
Collaborator

I have run into this too, and it's very annoying (and a bit scary, because often it's hard to see which commands were executed and whether they caused any damage).

It might be possible to use tcell's EventPaste to distinguish pasting from typing. Support for it would first have to be added to gocui, and passed to our views somehow. (I don't think it will work on Windows.)

Alternatively, we might use some timing heuristic to distinguish pasting from manual typing. If "enter" was hit very fast after the last character typed, it must have been a paste, and we switch to the description instead of closing the dialog. I'd be curious if we can find a threshold that works reliably enough (for people who type very fast). This doesn't help if the first character in the clipboard is a newline, but maybe that's no so common.

This happens to me frequently when I paste the contents of the "copy commit message" shortcut.

Same here. This, however, could be mitigated by adding a "copy commit subject" command, since often it is only the subject that I want to paste. I have sometimes missed this option for other reasons too. The only challenge is to come up with a keybinding for it, as s is already taken for "copy SHA".

@mark2185
Copy link
Collaborator

mark2185 commented Dec 8, 2023

Could we intercept ctrl+v/ctrl+shift+v and invoke appendToDescription(clipboard string)?

@stefanhaller
Copy link
Collaborator

Could we intercept ctrl+v/ctrl+shift+v

I don't think so. On Mac that's Command+V, and it's handled by the terminal application before the running program even sees it.

@OliverJAsh
Copy link
Author

This happens to me frequently when I paste the contents of the "copy commit message" shortcut.

Same here. This, however, could be mitigated by adding a "copy commit subject" command, since often it is only the subject that I want to paste. I have sometimes missed this option for other reasons too. The only challenge is to come up with a keybinding for it, as s is already taken for "copy SHA".

Agreed! I would like this too.

@stefanhaller
Copy link
Collaborator

Wanna give it a try? It should be relatively simple to add.

For the keybinding, I guess we could consider making the breaking change of renaming "Copy SHA" to "Copy hash", change its keybinding to h, and then use s for "Copy subject". Yes, this will badly break it for people who have y s in their muscle memory, but then they should probably switch to <c-o> anyway. Any thoughts about that?

@niralmaruda
Copy link

Hello @stefanhaller,

I have just started learning GO. I liked your project and I would love to contribute and be a part of it. I say this issue open since Dec 8, 2023. If anyone is not working on it, Do you think I can work on it? It might take some time but I would like to learn and gain experience from this contribution.

@stefanhaller
Copy link
Collaborator

@niralmaruda What exactly were you thinking about working on? The discussion above mentions three potential solutions; one has been implemented already. The remaining two both seem very tricky to me, and probably not really suitable for a beginner. Which of these are you interested in?

@niralmaruda
Copy link

niralmaruda commented Aug 19, 2024

@niralmaruda What exactly were you thinking about working on? The discussion above mentions three potential solutions; one has been implemented already. The remaining two both seem very tricky to me, and probably not really suitable for a beginner. Which of these are you interested in?

Ohh, If that's the case then I should come back to this when I have a strong understanding of this repo. Until then I should look for some beginner-friendly issues. If you are aware of any issue like that can you please point me towards that @stefanhaller ?

@stefanhaller
Copy link
Collaborator

Here's a PR that should fix this: #4234. Testing welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants