Skip to content
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

.SelectedFile.Name not recognized in commitFiles context #3155

Closed
tonyvitonis opened this issue Dec 10, 2023 · 14 comments
Closed

.SelectedFile.Name not recognized in commitFiles context #3155

tonyvitonis opened this issue Dec 10, 2023 · 14 comments
Labels
bug Something isn't working

Comments

@tonyvitonis
Copy link

Describe the bug
When I try to use {{ .SelectedFile.Name }} in a custom command in the commitFiles context, I get this error:

template: template:1:96: executing "template" at <.SelectedFile.Name>: nil pointer evaluating *models.File.Name

To Reproduce
Steps to reproduce the behavior:

  1. Add a custom command.

customCommands:

  • key: D
    context: commitFiles
    command: 'git difftool {{ .SelectedLocalCommit.Sha }}^..{{ .SelectedLocalCommit.Sha }} -- {{ .SelectedFile.Name }}'
    subprocess: true
  1. Open lazygit.
  2. Tab to commits list.
  3. Hit Enter to open Diff files.
  4. Select a file.
  5. Hit D.
  6. See the error mentioned in the bug description above.

Expected behavior
See git difftool (in my case, vimdiff) invoked for the selected file.

Screenshots
Not applicable?

Version info:
lazygit --version
commit=cf82e69bbe8ec25facbc354fe61c7ad8a5413ea1, build date=2023-12-09T14:40:36Z, build source=unknown, version=cf82e69b, os=linux, arch=amd64, git version=2.43.0

git --version
git version 2.43.0

Additional context
I tried running lazygit --debug and lazygit --log. Nothing showed up in the log at the time of the error.

Note: please try updating to the latest version or manually building the latest master to see if the issue still occurs.

@tonyvitonis tonyvitonis added the bug Something isn't working label Dec 10, 2023
@stefanhaller
Copy link
Collaborator

That's not a bug, . SelectedFile is only valid in the "files" context, it's unset in the "commitFiles" context. Try .SelectedCommitFile.Name instead.

You can also use .SelectedCommitFilePath instead, the difference is that this also works for directories. However, it will then present the diffs for every file in that directory one after another, and it's hard to cancel out of that if there are many.

I also just opened a PR for adding difftool support to lazygit, see #3156. I'd be interested in your feedback if you want to test it. I made it so that --dir-diff is passed to the difftool command if you select a directory, which may or may not be desirable depending on how well your external diff tool supports that. Let me know what you think. I also bound it to <c-t> instead of D, since I want it to work both in the files context and the commitFiles context, and D is already taken in the files context.

@tonyvitonis
Copy link
Author

.SelectedCommitFile.Name works. Thanks for the quick response.

I'd be happy to test the difftool support. How do I go about getting it?

@stefanhaller
Copy link
Collaborator

By building from source. Install go, clone this repo, check out the difftool branch, and do make install.

@tonyvitonis
Copy link
Author

This works as expected on the individual files, thanks.

When I hit Ctrl-T with a directory selected, I get what looks like a netrw diff. I'd have expected to see git difftool run against every committed file in the directory.

Also, sometimes even with large commits, I like to run a git difftool on the repository root directory. Is there a way to do that?

@stefanhaller
Copy link
Collaborator

When I hit Ctrl-T with a directory selected, I get what looks like a netrw diff. I'd have expected to see git difftool run against every committed file in the directory.

Yeah, that's because of the --dir-diff option, as I mentioned above. I suppose we'll have to make it configurable if some people like it and others don't (personally I don't care, as I never use difftool on directories). But I wonder if we then also have to make the --prompt option configurable? I chose to hard-code --no-prompt in my branch, but without --dir-diff the --prompt option would make it easier to cancel out of the sequence of individual files if there are hundreds and you want to stop after a few.

Also, sometimes even with large commits, I like to run a git difftool on the repository root directory. Is there a way to do that?

Currently not. There's also no way to look at and scroll through the entire diff in lazygit's diff view, something I occasionally miss. But I don't have a good idea for a UX to support this. We could add an additional, artificial hierarchy level for the root so that you can select it, but it would feel a bit strange to be able to collaps it (why would you?).

@tonyvitonis
Copy link
Author

tonyvitonis commented Dec 11, 2023

I'm sure you'll come up with something good. Lazygit is a great app. Thanks again.

BTW, when I say "the repository root directory", I just mean where the .git directory is, as in git rev-parse --git-dir. I often like to diff all of the files for a given commit.

@stefanhaller
Copy link
Collaborator

BTW, when I say "the repository root directory", I just mean where the .git directory is,

Yes, I got that. For commits this could be achieved by offering the difftool command not only in the commitFiles context, but also on a commit itself, this would be nicely analogous to how lazygit's diff view shows the diff for the entire commit on the right side. However, this doesn't help when you want to do it for all modified files in your working copy.

@tonyvitonis
Copy link
Author

One more thing: I'm unclear on how changes like this get into the master branch. Is it just a matter of waiting for it to happen?

@tonyvitonis tonyvitonis reopened this Dec 21, 2023
@stefanhaller
Copy link
Collaborator

One more thing: I'm unclear on how changes like this get into the master branch. Is it just a matter of waiting for it to happen?

That's a good question. For this particular case, there are still open questions that need to be answered somehow, and it's unclear how. My comments above were meant as a call for input or discussion (maybe that wasn't clear enough), but all I got was "you'll come up with something good". That doesn't help, really. 😉

In other cases it's often Jesse who just makes a call after hearing all the opinions. He's not around right now (temporarily), so that doesn't work here. I suppose I could just make a decision, but it would likely be to keep the existing behavior (pass --dir-diff), for simplicity, and it doesn't sound like you would be happy with that.

@tonyvitonis
Copy link
Author

Ah! I didn't know you were looking for more from me.

Here's what I'd like to see:

  • Hit Ctrl-T on a highlighted file name, run git difftool on that file. In the Files window, it'd be for the changes in the working directory. In the Branches windows, it'd be at HEAD. In the Commits window, it'd be at that commit.
  • Hit Ctrl-T on a highlighted directory name, run git difftool on that directory. Context same as for files.
  • In both cases there's no need to change the items that are displayed. If I want to do a difftool for the whole repo, it's easy enough to do it at the command line.

Does that help?

@stefanhaller
Copy link
Collaborator

Does that help?

Well, not really. 😄 The main open question was related to whether we show a directory diff (--dir-diff) when a folder is selected, or present the individual file diffs one after the other. You said you expected the latter; I prefer the former. We could make this configurable, but then we probably also need to make the --no-prompt option configurable, because without it it's difficult to cancel out of a sequence of hundreds of file diffs. But with --prompt the user experience is terrible, and it really depends on the external diff tool whether it's needed.

However, that's too many config options, I'd like to avoid adding more than necessary, because the lazygit config system is already unwieldy. So I lean towards keeping it the way it is (with --dir-diff).

How well this works really depends on the diff tool used. Personally I use Beyond Compare, it does a pretty good job at showing a directory diff. I just tried FileMerge (on Mac, comes with Xcode), and it's terrible. I don't have access to many other diff tools that I could try, so that's one more aspect for which input would be useful.

@tonyvitonis
Copy link
Author

--dir-diff is fair enough. I work on a mainframe product that already has too many options as it is, so I understand completely. Ship it!

(One way I could go about getting what I need is to make a local branch off of master with my changes to it. Doesn't sound hard.)

@stefanhaller
Copy link
Collaborator

--dir-diff is fair enough.

OK thanks, that makes it easier.

Ship it!

It's not quite as easy. The PR still needs a code review, and it doesn't have tests.

(One way I could go about getting what I need is to make a local branch off of master with my changes to it. Doesn't sound hard.)

I've done this many times, and yes, it can work well, but it can also be annoying at times when you continuously rebase your local stuff onto upstream master and keep getting conflicts. If you're lucky it doesn't happen too often.

@tonyvitonis
Copy link
Author

"Ship it!" is an old mainframe quasi-joke that deliberately ignores all of the reviewing and testing and administrative work that everyone knows needs to be done. :)

Thanks again for all your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants