-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Improve clicking in diff view to enter staging/patch building #3986
Comments
I find the idea of using a custom OSC protocol to annotate each line of the diff very attractive, the more I think about it. If it works out, we can encourage other pagers to support it, or other host applications (not sure there are any?) to use it. This would be somewhat similar to OSC 8 for hyperlinks. In lazygit, it would allow us (provided we find a way to focus the main view; more on that in #3988) to select a line in the diff, and either hit enter to go directly to the staging view, or When using no pager (or a pager that only colors things) we can parse the diff ourselves to work out what file and line was clicked on, so we can offer the same features to all users. @dandavison Would you be interested in collaborating on this? |
Hi @stefanhaller I appreciate the careful description of this problem and proposal. I'm being slow to understand it, but that is my fault.
I thought that delta added hyperlinks to the line numbers of all lines except removed lines. What is a line "between hunks"? This might help make sure we're on the same page: https://github.com/dandavison/delta/blob/main/ARCHITECTURE.md#handling-diff-hunk-lines |
When a single file (not a directory) is selected in the files panel, you can click in the diff on the right to enter staging, with the clicked line selected. This is very convenient, but there are several problem with this:
This last one is really a bummer. It is a very common use case for me to review my branch in the commits panel, and then I notice something that I want to improve; for example, I realize that a certain diff hunk should be extracted into a separate commit. To do that, I have to hit enter to go into the commit files view, then find the file again that I was just looking at, hit enter again to go into patch building mode, and then find the hunk again that I was just looking at. This is very cumbersome if there are many changed files in the commit, or many diff hunks in the file in question. It would be so much easier to just click in the diff view of the commit and go straight into patch building mode, with the clicked line selected.
(Side note: for editing a given line in the diff we recently improved this by using delta's hyperlink feature, so you can now jump straight to the editor by clicking on a line number. This is great, and I use it all the time; and this is a workaround for cases where you don't absolutely need a custom patch. For example, when I see a left-over printf statement from debugging that I want to drop from a commit, I now jump to the editor, delete it there, and amend the change into the commit I'm looking at. It should be as easy to make a custom patch and drop it, though.)
To implement all this, we can use delta's hyperlinks again, but it's a bit of a hacky solution. When clicking on a line in the diff (but not on a hyperlinked line number) we can examine the rest of the line to see if there are any
lazygit-edit:
URLs in it; if so, we use it to determine which file and line we're on. We can then select that file in the tree view on the left, and work out the line number in the patch that corresponds to the line number in the file.I made a draft PR that implements this, and it works reasonably well. The main problem is that not all lines in the diff have a hyperlink; the ones between hunks don't, and deleted lines don't, either. As a user you need to learn to click on a line that has an underlined line number, which is not intuitive.
@dandavison I wonder if we can improve this by adding a mode to delta where it adds "invisible" hyperlinks to all lines in the diff, not just added or context lines. One way to do this could be to add a space at the beginning or end of the line and attach the hyperlink to that, and follow it with a
\b
character so that it's invisible. Very hacky, yes, but should improve things a bit.Alternatively, we could use custom, private OSC messages to annotate all lines with file/lineNo information in some custom format. Probably a bit cleaner. Let me know if you'd be interested in exploring this more, or if you have other ideas how to solve this.
@jesseduffield Keen to hear your thoughts on all this.
The text was updated successfully, but these errors were encountered: