Expose selected commit ranges to custom commands

[Mic92]

Is your feature request related to a problem? Please describe.

Currently we can access a single selected commit via {{.SelectedCommit.Sha}}.
However using the v key it's also possible to select multiple commits.
I.e. this allows to create a pull request from a range of commits or doing other types of automation that require access to multiple commits.

Describe the solution you'd like

A potential API could look like thi
s:
{{.SelectedCommits.StartSha}} {{.SelectedCommits.EndSha}}

Describe alternatives you've considered

Can't think of a different way

From my understanding this needs to extend

https://github.com/jesseduffield/lazygit/blob/master/pkg/gui/services/custom_commands/session_state_loader.go#L203

The only thing that is unclear to me is what should happen if the user has not selected anything?

[stefanhaller]

I started a branch to implement this a while ago, but I kind of lost momentum on it and worked on other things first that I found more important. I can try to take this up again and make some progress.

A potential API could look like this: {{.SelectedCommits.StartSha}} {{.SelectedCommits.EndSha}}

No, I think we should just expose .SelectedCommits as a slice, and then you can use go's template syntax to do all sorts of useful things with it. It will need some documentation and examples for those who are not familiar with it.

[Mic92]

Do you still have the branch?

[stefanhaller]

Looking at it again it turns out that I didn't get very far yet. I pushed it here if you want to have a look. Feel free to take it from there, but please let me know if you do so that we don't both spend time on it.

So far I only exposed SelectedCommits, so the same needs to be done for the other model things that support range selections (which is all of them, I think).

But I also think that writing the code to expose things is not the challenge here; the challenge is rather to write enough documentation or provide examples to make this easily usable. Here's a simple custom command that I used for testing, it simply outputs the hashes of the selected commits to an output panel, separated by comma:

    - key: "X"
      command: '{{$sep := ""}} echo "{{- range .SelectedCommits}}{{$sep}}{{.Hash}}{{$sep = ", "}}{{- end}}"'
      context: "global"
      description: "Test SelectedCommits"
      showOutput: true

Here's the documentation of the template syntax: https://pkg.go.dev/text/template.

[jesseduffield]

For what it's worth I think it's fine to expose both a slice of selected commits and a start commit and end commit. Of course, there will be times where start and end are a bit nonsensical like when merges are involved but I don't think that's a good enough reason not to have it.

[stefanhaller]

For what it's worth I think it's fine to expose both a slice of selected commits and a start commit and end commit.

Hm, I was about to say that's not necessary because you can get those via go's template syntax; however, that only seems easy for the start commit ({{index .SelectedCommits 0}}), I couldn't find an easy way to get the last element.

So yes, exposing First and Last on the exposed slice seems like a good idea. I pushed a fixup to my branch (a1a6b97) that does this, you can now use the range of hashes like so: command: 'echo {{.SelectedCommits.First.Hash}} {{.SelectedCommits.Last.Hash}}'. We can easily expose other useful things on the slice this way if we need to.

We probably want to introduce some generics for this if we do this for all model slices.

[jesseduffield]

Another thing to consider: you might select a range from the top-down or from the bottom-up (just based on where the cursor starts) but the first and last commits (in terms of what you selected first/last) are not as important as the top and bottom commits (especially if you feed them into a command that wants to make use of the diff between them). @stefanhaller does the code you linked treat the top-down and bottom-up direction the same?

[Mic92]

Intuitively I would say "start" should point to the oldest commit and "end" to the newest.

[stefanhaller]

@stefanhaller does the code you linked treat the top-down and bottom-up direction the same?

Yes; First is always the top commit, and Last is the bottom commit, no matter in which direction the range was selected.

Intuitively I would say "start" should point to the oldest commit and "end" to the newest.

Hm, interesting; it's the other way round in my branch. I was just going by what you see in the view, that felt the most intuitive to me.

[Mic92]

Git's range syntax also goes from the oldest to the newest commit: git diff HEAD~1..HEAD
That was my thought process behind it. And most tools first will need to apply to oldest patches. But honestly I am happy with whatever I am getting :).

[stefanhaller]

Yes, I understand that that's where the suggestion was coming from. I would just find it weird if SelectedCommits is the only container that has First and Last swapped compared to all the other ones.

But then again, in all other containers the concept of First and Last is probably not useful at all, because you can't have a range of branches or files. So maybe it would make sense to expose it only for Commits? And then I don't care much which order we decide on. Happy for @jesseduffield to make a call.

[jesseduffield]

I agree: commits are special and it's only for commits that we ever need to specify the first and last. Given git does start..end, let's make it so that the start commit is the bottom commit and the end commit is the top commit. And I'd call it CommitRangeStart and CommitRangeEnd

[stefanhaller]

Why would you make them separate variables rather than properties of SelectedCommits? I find it strange that CommitRangeStart doesn't have the word "selected" in it like the other commit-related things we expose.

Also, the two of you keep talking about "start" and "end"; I was trying to avoid these because for "end" it is always unclear whether it's inclusive or exclusive. As a C++ programmer I'm used to "end" meaning "the first element after the end of the range". "First" and "Last" don't have this problem.

[jesseduffield]

Ah yes. How about SelectedCommitRange with .From and .To? I don't like using SelectedCommits directly because we may actually want that in future to just be a slice and in this case we want to provide a specific from/to value.

Link to the chatGPT convo that led me to this conclusion: https://chatgpt.com/share/6790348f-6438-8011-93b8-9d396aaf4dcc

[stefanhaller]

I don't like using SelectedCommits directly because we may actually want that in future to just be a slice

It is a slice in my version. I’m just adding the First/Last properties to it. I like this best because we expose fewer separate top-level things this way.

As for To/From vs. First/Last: ChatGPT's main reason against First/Last is that commits form a graph and there is no strict start/end ordering there. That's true, but we are talking about the linear (flattened) list of commits in lazygit's view here; that's where I select my commits, and there's certainly a very well-defined first and last commit in my selection. That was also my reason to make First the top one and Last the bottom one; I just wanted to model what I see in my view.

Also, you were explicitly asking ChatGPT about diff terminals, but the feature will be used for other things that are quite different from diffs. For example, you could implement a copy-paste feature between repos using

customCommands:
  - command: 'git format-patch --stdout {{.SelectedCommits.Last.Hash}}^..{{.SelectedCommits.First.Hash}} | pbcopy'
    context: "commits"
    description: "Copy selected commits to clipboard"
  - command: 'pbpaste | git am'
    context: "commits"
    description: "Paste selected commits from clipboard"

I'm not really hung up on First/Last or on the order, I'm open to changing both, but I'm just saying that I still find what I currently have to be the most intuitive representation (First being the top selected commit, Last being the bottom one).

[stefanhaller]

Ok, I'm being a bit slow as usual. 😄 But I'm now back from my morning run, and it's always amazing how a good dose of oxygen can clear up the foggy confusion in my head.

So, what I said above about looking at lazygit's view isn't really relevant. What matters is just whether we model things as a slice or as a range object. If we expose the selected commits as a slice and then add properties to it like I did in my branch, then the best names for those properties are First and Last, and First must be the first element of the slice. Swapping them doesn't make any sense at all, and To/From wouldn't be names that mean much in this case.

But if we expose a separate range object, we can think about what its fields really mean, and I agree that To/From are the best names, and that they should be ordered with From being the oldest one.

So it all boils down to whether we expose a slice or a range, or both.

Now, for commits I don't actually see any use cases for wanting the slice of all selected commits; you probably only want to use the range in that case. For other containers it's probably the other way round. But exposing slices for those raises some questions; e.g. for files, do you always want the slice of all selected entries, or do you sometimes only want the files or only the directories? Or do we need to do some cleanup of the slice first to eliminate children of selected directories, like we do with normalisedSelectedNodes in files_controller.go? For reference, a request for exposing range selections of files is here, but it doesn't say very much about these questions.

So maybe the best way to proceed for now is to expose SelectedCommitRange with .From and .To like you suggested, and don't expose any slices for now. That's easy to do and a good increment in user value. We can then later think about how to expose slices for other things if and when the need arises.

[jesseduffield]

So maybe the best way to proceed for now is to expose SelectedCommitRange with .From and .To like you suggested, and don't expose any slices for now. That's easy to do and a good increment in user value. We can then later think about how to expose slices for other things if and when the need arises.

I agree, I think that's going to solve the typical use case for a commit range.

With respect to the file view, we could just provide multiple options e.g. a slice of all file paths, a slice of all paths with overlaps excluded (i.e. if a directory is selected in its entirety, pass that path and don't pass the paths of any descendants).

It actually sounds like a generic slice would only really be useful for the branches view

[stefanhaller]

Ok cool. Here's a PR: #4204.

Not going to work on file paths for now, since I don't have a need for this myself.