lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

SelectedSubCommit is nil in the commits tab

Open abelmul opened this issue 1 year ago • 4 comments

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
  1. Go to commits tab
  2. Click on any commit
  3. Scroll down to a file
  4. Click the O key
  5. 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

abelmul avatar Jun 14 '24 19:06 abelmul

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?

stefanhaller avatar Jun 15 '24 14:06 stefanhaller

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)

jesseduffield avatar Jun 24 '24 11:06 jesseduffield

@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.

stefanhaller avatar Jun 29 '24 07:06 stefanhaller

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

jesseduffield avatar Jun 29 '24 07:06 jesseduffield