jesseduffield / lazygit

simple terminal UI for git commands
MIT License
47.94k stars 1.72k forks source link

SelectedSubCommit is nil in the commits tab #3663

Open abelmul opened 2 weeks ago

abelmul commented 2 weeks ago

Describe the bug Title

To Reproduce

  1. Add a custom command like
    - key: O
    context: commitFiles
    description: open files in commit
    command: $EDITOR -c 'Gedit {{.SelectedSubCommit.Sha}}:{{.SelectedCommitFile.Name}}'
    subprocess: true
  2. Go to commits tab
  3. Click on any commit
  4. Scroll down to a file
  5. Click the O key
  6. See error

Expected behavior SelectedSubCommit should not be null

Screenshots image_2024-06-14_22-19-28

Version info: commit=v0.42.0, build date=2024-05-19T10:54:29Z, build source=binaryRelease, version=0.42.0, os=linux, arch=amd64, git version=2.45.1

stefanhaller commented 2 weeks ago

SelectedSubCommit is for when you hit enter on a branch or tag to look at its commits. If you use your custom command on the commit files of one of those, it will work. For normal commits, you want SelectedLocalCommit.

Now, it is a problem that the commitFiles context is used for all three cases (local commits, reflog commits, and subcommits), so you would have to use different template variables in each of these situations, but it's impossible to make the distinction.

I'm actually wondering why we need different variables SelectedLocalCommit, SelectedReflogCommit, and SelectedSubCommit. Wouldn't it be better to only have a single SelectedCommit variable, and decide based on context what model object to set it to? Am I missing something @jesseduffield?

jesseduffield commented 1 week ago

I agree. It would be good to do the same with SelectedPath (which currently only refers to the files panel and does not include the commit files panel).

Of course for backwards compatibility we'd want to keep the existing keys (though I think it's fine to repurpose SelectedFile and SelectedPath without introducing new keys for those)

stefanhaller commented 3 days ago

@jesseduffield This works for SelectedPath (since it's a string), but it's not so straightforward for SelectedFile, because the types are different (File vs. CommitFile). Are we ok with SelectedPath returning different types depending on context? We might want to do the same for SelectedLocalBranch vs. SelectedRemoteBranch, and just use SelectedBranch for both, but with different types.

This would just be for cosmetics, since it's not possible to define a single custom command that works for both local branches and remote branches (although I sometimes wished it was possible to say context: "localBranches,remoteBranches").

The only case where it is really needed to make things work is SelectedCommit, as far as I can see.

jesseduffield commented 3 days ago

Ah interesting. Yeah if we're dealing with two different types, then let's just leave those as-is. If we're dealing with a string let's be general. For the record I bet that users only really care about branch names, commit hashes, and file paths (all strings).

This would just be for cosmetics, since it's not possible to define a single custom command that works for both local branches and remote branches (although I sometimes wished it was possible to say context: "localBranches,remoteBranches").

I agree