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

Add multiple selection in diff view #231

Merged
merged 3 commits into from
Aug 19, 2020

Conversation

cruessler
Copy link
Collaborator

This is a first attempt at implementing multiple selection in the diff view.
You can use Shift+ArrowUp as well as Shift+ArrowDown to extend the
selection.

Known issue: If the selection spans more than one hunk, only the hunk that
contains the selection’s start is selected. I don’t know a good way to address
that. Preventing the selection from spanning more than hunk seems like an
obvious choice, but then, of course, you could only copy lines out of one hunk
at a time.

Still left to do is the actual copying to clipboard, but that should be fairly
easy once the above issue is resolved.

Closes #229.

@extrawurst
Copy link
Collaborator

looks already like a great start. only two minor things:

  1. is it intended that when u do both up and down that it simply grows the selection in both directions instead of moving the bottom like usual?
  2. similar to 1) I think the scrolling should still work and go with the movable part of the selection
  3. I think after stopping to do selection up/down without shift it should land where the movable part of the selection was

@cruessler cruessler force-pushed the extend-selection-in-diff-view branch from 665098d to bb8a50a Compare August 12, 2020 07:57
@cruessler
Copy link
Collaborator Author

I have updated the PR.

ad 1) I found the initial behavior unexpected, too. I changed it so that Shift+Arrow now only moves the selection’s end which is closer to what you would expect if you come from vim.
ad 2), 3) I have not yet addressed that, but I’ll keep it in mind.

Doing the actual copying seems simple. At least on my machine (Ubuntu 20.04, X11), it worked immediately. Coming from vim, I have bound copy to y, but that can of course easily be changed.

@extrawurst
Copy link
Collaborator

@cruessler I checked out the changes. works better now, but I would like to be able to also scroll up beyond the start selection line (basically inverting start/end selection index).
also I can confirm the copy to cb works on Mac too👍

@cruessler cruessler force-pushed the extend-selection-in-diff-view branch from bb8a50a to 8e1ac88 Compare August 17, 2020 17:25
@cruessler
Copy link
Collaborator Author

cruessler commented Aug 17, 2020

@extrawurst I just updated the PR with another round of changes! You can now move the end past the start, and the content of the window scrolls, so that the end is always visible.

Copy link
Collaborator

@extrawurst extrawurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey man, this is getting close. Sorry for another round of feedback. Looks pretty good already. Hope those last few are not a big deal anymore. Love to merge this!❤️

@cruessler cruessler force-pushed the extend-selection-in-diff-view branch from 8e1ac88 to ecd35f5 Compare August 18, 2020 16:32
@extrawurst extrawurst marked this pull request as ready for review August 18, 2020 20:27
@extrawurst
Copy link
Collaborator

@cruessler aside the linux question, can you rebase once more onto master, so we see if the windows pipeline (that got fixed now) runs through?

- Draw `msg` after `inspect_commit_popup` to make sure the error message
  is visible
- Move `try_or_popup!` to `utils`
@cruessler cruessler force-pushed the extend-selection-in-diff-view branch from ecd35f5 to a436b6c Compare August 19, 2020 18:03
@extrawurst extrawurst merged commit bc23270 into gitui-org:master Aug 19, 2020
@extrawurst
Copy link
Collaborator

Thanks so much @cruessler - great work!

@cruessler
Copy link
Collaborator Author

@extrawurst One addition: I just saw that there are packages for Fedora and Arch. It is possible that they might need to be adapted to support clipboard access, but I am not familiar with building packages, so I don’t know how they work.

@extrawurst
Copy link
Collaborator

@ignatenkobrain maintains fedora and @wezm did AUR maybe you guys can chip in before we release this and ruin your day? :)

@wezm
Copy link
Contributor

wezm commented Aug 19, 2020

Looks like there's a couple of new runtime dependencies (xcb and friends). That's no big deal. 👍

@ignatenkobrain
Copy link
Contributor

I think it is fine for Fedora, but right now we are not able to build gitui because there is some bug that makes rustc to segfault on ppc64le.

@cruessler cruessler deleted the extend-selection-in-diff-view branch May 13, 2021 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selection in diff to copy lines to clipboard
4 participants