Non-sticky range diff

[stefanhaller]

I often want to see the diff of a range of commits. The two main use cases are:

• I want to see a combined diff of all the commits in my branch to quickly get an overview over which files I touched.
• I'm considering squashing two or more commits, but I'm not sure yet if it's really better; I want to see the resulting diff of both commits to help decide. As a workaround, I could simply squash them, and when I'm not happy with the result, hit z to undo; however, that doesn't work well with stacked branches.

Both of these can be solved today using diffing mode (navigate down to one end of the range (one after the end, that is), hit shift-W, navigate up to the other end). This results in exactly what I want to see (including being able to hit enter to see the files of the diff, even being able to create a custom patch from it). However, this is too many key-presses, it's not very easy to discover, and it's too sticky (it happens too often that I forget I'm in this mode, and then I move the selection around looking at diffs that make absolutely no sense).

I would find it totally intuitive if, whenever I range-select several commits (either a sticky range using v, or a non-sticky range using shift-arrow), I would see a diff of the range of those commits. This would perfectly match the general concept of "show me details of whatever is selected in the side panel", and it would solve the use cases above perfectly for me. I also want to be able to hit enter to work on the files of this range diff, like in diffing mode.

Other git clients have a similar feature, but it works slightly differently:

• Fork requires you to make a non-contiguous multiselection of two commits (using command-click, something that lazygit doesn't support). If you select more than two, including a contiguous range of commits, it shows an error, asking you to select exactly two. Also, it requires you to select one commit below the beginning of the range (like lazygit's shift-W mode), which I find unintuitive.
• Sublime Merge is like Fork, except that it also allows a contiguous range of commits. This is more convenient than Fork because it allows selecting with the keyboard (using shift-down), whereas Fork's non-contiguous selections can only be made with the mouse. But like Fork, it requires selecting one below the beginning of the range.

I understand that selecting one commit below the start of the range makes sense because it is closer to what you do on the command line. If you have commits A, B and C, then git diff A..C displays the combined diff of B and C, but doesn't include A's changes. That's what Fork and Sublime Merge (and shift-W in lazygit) try to emulate, it seems. But for a range selection of commits I find this very unintuitive: I want to see the combined diff of all the commits that I have selected, so I'm strongly proposing to do a git diff A^..C if A, B, and C are selected.

Note that I'm not proposing to replace diffing mode with this new non-sticky diff mode; diffing mode is probably still useful for diffing two things from different panels, e.g. a tag against a commit, or a commit against a branch.

I'm pretty far along implementing this, but I have a few questions that I'm unsure about:

1. It is unclear to me what to do if a range of reflog entries or stash entries is selected. These types of objects don't form a linear sequence, so it doesn't make sense to display a range diff for them. We could either keep the behavior of master for these cases (i.e. show the diff for the free end of the selection), or show a message such as "Cannot display diff for range of stash entries".
2. A similar situation arises when selecting a range of rebase todos in an interactive rebase. Since "pick" entries can be reordered, we'd be showing a diff for the range of those commits in their original position before the rebase, which could be very confusing. Same options as in 1., either use the single selection, or show a message that we can't show this diff.
3. Finally, what if there is a range selection when we are also in diffing mode? It gets very confusing here. It might be best to keep the master behavior in this case, and diff the free end of the selection against the diffing mode anchor. But again, we could consider showing an error message instead.

[stefanhaller]

After re-reading this I realize that my use of the term "non-sticky" could be confusing here. I'm using it for describing this new lightweight ad-hoc range selection diff mode, as opposed to the sticky shift-W mode. However, we also use sticky vs. non-sticky for the difference between a v range selection and a shift-arrow range selection, and this new non-sticky diff mode works for both. Confusing...

[jesseduffield]

I like this idea a lot. I also share your intuition that it's better to do ^A..B than to do A..B but it does kinda lock us in to that pattern. If we used A..B everywhere then the logic is simple and consistent: it can be applied in any view that contains commits including the reflog view and the divergence view (e.g. when comparing two commits that are from the two different branches).

But, we can just say that that if you want A..B, you can do a sticky diff.

In answer to your questions:

It is unclear to me what to do if a range of reflog entries or stash entries is selected. These types of objects don't form a linear sequence, so it doesn't make sense to display a range diff for them. We could either keep the behavior of master for these cases (i.e. show the diff for the free end of the selection), or show a message such as "Cannot display diff for range of stash entries".

If the purpose is to show the combined diff of all selected commits, then that's not something anybody will need in the reflog view, so I'd just stick to master's behaviour

A similar situation arises when selecting a range of rebase todos in an interactive rebase. Since "pick" entries can be reordered, we'd be showing a diff for the range of those commits in their original position before the rebase, which could be very confusing. Same options as in 1., either use the single selection, or show a message that we can't show this diff.

Hmm. Wonder if there's a command to literally show a combined diff of N commits rather than specifying the two terminals. That might make this easier to implement for edge cases like that.

Finally, what if there is a range selection when we are also in diffing mode? It gets very confusing here. It might be best to keep the master behavior in this case, and diff the free end of the selection against the diffing mode anchor. But again, we could consider showing an error message instead.

Agreed on keeping the master behaviour in that case. I don't think an error is necessary: diffing mode takes precedent over the non-diffing mode diff and that can be true whether a range is selected or not.

[stefanhaller]

(e.g. when comparing two commits that are from the two different branches).

Good that you bring this up, I had missed that. This would also be an error condition like the others below.

But, we can just say that that if you want A..B, you can do a sticky diff.

I agree.

If the purpose is to show the combined diff of all selected commits, then that's not something anybody will need in the reflog view, so I'd just stick to master's behaviour

Would you be opposed to an error message though? I would find that less confusing, and I think it will also be easier to implement.

Hmm. Wonder if there's a command to literally show a combined diff of N commits rather than specifying the two terminals.

I don't think there is, and I'm also not sure how that could possibly work in case there are conflicts.

Agreed on keeping the master behaviour in that case. I don't think an error is necessary: diffing mode takes precedent over the non-diffing mode diff and that can be true whether a range is selected or not.

Same here, would you be opposed to an error in that case?

[jesseduffield]

I don't think there is, and I'm also not sure how that could possibly work in case there are conflicts.

Good point, I forgot that git's diffs are based on snapshots, not patches.

As for errors: with the reflog view it feels over-the-top to me: the fact we show a combined diff is a nice bonus imo. It seems weird that if you are in fact just using the multi-select to perform some bulk action, we're going to show an error in case you're actually trying to see a combined diff. With sticky diff mode, I think it's more appropriate to show an error because it would be especially strange to do anything with a range select in that case.

[stefanhaller]

As for errors: with the reflog view it feels over-the-top to me: the fact we show a combined diff is a nice bonus imo. It seems weird that if you are in fact just using the multi-select to perform some bulk action, we're going to show an error in case you're actually trying to see a combined diff. With sticky diff mode, I think it's more appropriate to show an error because it would be especially strange to do anything with a range select in that case.

Good point, this makes sense. Too bad, this will make the implementation trickier. I'll see if I can solve this somehow.

[stefanhaller]

PR is here: #3869.