Show the number of lines changed per file in working file tree view

jesseduffield / lazygit

simple terminal UI for git commands

MIT License

53.65k stars 1.87k forks source link

Show the number of lines changed per file in working file tree view #4015

Open johannaschwarz opened 1 month ago

johannaschwarz commented 1 month ago

PR Description Implements Issue #3643 Adds the number of line changes to the end of each file line in the Files view. Also adds the possibility for the user to enable and disable this feature through the UserConfig.
Please check if the PR fulfills these requirements
[x] Cheatsheets are up-to-date (run go generate ./...)
[x] Code has been formatted (see here)
[x] Tests have been added/updated (see here for the integration test guide)
[ ] Text is internationalised (see here)
[x] If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
[ ] Docs have been updated if necessary
[x] You've read through your own file changes for silly mistakes etc

stefanhaller commented 2 weeks ago

When I first briefly looked at it two weeks ago, my gut reaction was:

yes, looks nice, might be useful in some cases
it adds noise to the UI. Lazygit's UI is already quite noisy in some ways; I've observed new users not finding their way around it easily because it displays so much stuff
I'm worried about performance.

I haven't had time to come back to it since then, and have a closer look or even try it out myself, but currently my feeling is that I would keep it off by default. I'll see if I find some time this weekend to try it out, maybe this will change my mind. 😄

opus131 commented 2 weeks ago

Fantastic addition! Maybe also aggregate the counts for collapsed folders?

To address the concern regarding UI noise: maybe on by default with an option to disable it? OR even better, a keybinding to toggle the visibility.

VS Code has keybindings to toggle inlay-hints and code-lenses, and it's great to quickly show/hide extra-info on demand via a single keystroke.

mark2185 commented 2 weeks ago

I kinda agree with Stefan, I think I'd keep it off for the Staging panel, but I'd definitely like to have it for the commit panel when a commit is "focused" with Enter.

stefanhaller commented 2 weeks ago

I tried it for a bit now, and I have to say that I (personally) don't like it. The line changes information doesn't provide any value for me, I don't need to see it; but it makes the view considerably more noisy, so I'd definitely turn it off. So it does need to be configurable.

As to whether the option should be on or off by default, I don't have a strong opinion; I'd tend to make it off by default, like we do for the base branch divergence information in the branches view.

Also, there's a visual appearance problem related to the colors: the green/red coloring looks great for files that are either unstaged (white) or partially staged (yellow), but for staged files (green) there's not enough visual distinction between the file name and the +x. This is especially a problem when all files are staged. I don't have a constructive suggestion how this could be improved, just flagging it as a problem.

jesseduffield commented 2 weeks ago

Let's make it off-by-default. See this twitter thread for some user feedback.

Also, there's a visual appearance problem related to the colors: the green/red coloring looks great for files that are either unstaged (white) or partially staged (yellow), but for staged files (green) there's not enough visual distinction between the file name and the +x. This is especially a problem when all files are staged. I don't have a constructive suggestion how this could be improved, just flagging it as a problem.

I think this is a minor thing which doesn't need fixing.

johannaschwarz commented 2 weeks ago

Alright, let's do it that way. I also feel there are valid points for keeping it off by default.

I appreciate your input! I'll incorporate your feedback when I find the time.

Another point: @mark2185 mentioned the feature would be useful when focusing on a commit. This is not part of my current implementation. Should I add it?

stefanhaller commented 2 weeks ago

Another point: @mark2185 mentioned the feature would be useful when focusing on a commit. This is not part of my current implementation. Should I add it?

I think this would be good, I could see this as being more useful than in the Files panel. I think it should be a separate config option, people might want to turn it on for Commit Files but off for Files. And it could be done in a separate PR, of course.