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

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

Merged
merged 1 commit into from
Dec 8, 2024

Conversation

johannaschwarz
Copy link
Contributor

screenshot
  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

Copy link
Owner

@jesseduffield jesseduffield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really great. I support enabling this by default. I'm even open to not making it configurable. I've asked on twitter if anybody would want to disable it: https://x.com/DuffieldJesse/status/1855137711667167267

I've left some comments.

Keen to get your thoughts on this @stefanhaller @mark2185

pkg/commands/git_commands/config.go Outdated Show resolved Hide resolved
@@ -100,6 +155,16 @@ type FileStatus struct {
PreviousName string
}

func (fileLoader *FileLoader) gitDiffNumStat() (string, error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three things:

  1. Just confirming that this can't be bundled into the git status command?
  2. How expensive is this command to run? If it's expensive, we may need to run it asynchronously
  3. I've noticed that this doesn't include untracked files. Is there a way to include those?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I did research on this and could not find any way to use git status for this.

  2. I tested this for 18 files changed with about 6000 insertions and 3000 deletions:

    • on my M1 Pro the git diff --numstat command takes 62 milliseconds.
    • in comparison: the git status command takes 46 milliseconds.

    What do you think performance-wise? Is it fast enough to run synchronously?

  3. According to my research it is not possible to include untracked files in a git diff command. I don't think it is super important to include these, but it would probably be possible to determine the number of added lines another way for untracked files.

pkg/gui/presentation/files.go Outdated Show resolved Hide resolved
@stefanhaller
Copy link
Collaborator

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
Copy link

opus131 commented Nov 9, 2024

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
Copy link
Collaborator

mark2185 commented Nov 9, 2024

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
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Collaborator

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.

@johannaschwarz
Copy link
Contributor Author

@jesseduffield any chance you can have another look? 😄

Copy link
Collaborator

@stefanhaller stefanhaller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I didn't realize this was ready for another review.

This is great work, looks very good to me. I found a few nits to pick (see below), but nothing major.

My biggest issue is the commit history, which is a bit of a mess tbh. First of all, please don't merge master into a feature branch; rebase onto master instead. And then, I like commits that are self-contained and atomic, so that the chain of commits tells a logical story. In this case, there might be one commit that adds the model, config, and file loader, and then a second commit that makes use of it in the UI. But you might also squash everything into just one commit, I guess.

pkg/commands/git_commands/file_loader.go Outdated Show resolved Hide resolved
docs/Config.md Outdated
@@ -164,6 +164,9 @@ gui:
# This can be toggled from within Lazygit with the '~' key, but that will not change the default.
showFileTree: true

# If true, show the number of lines changed per file in the Files view
showNumberOfLineChangesFilesView: false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add "in" here, i.e. showNumberOfLineChangesInFilesView?

(I was also wondering if we should shorten this to showNumstatInFilesView, given that --numstat is the git option that is being used. But maybe that's too awkward in camel case.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to showNumstatInFilesView. I like the shorter Version.

pkg/gui/presentation/files.go Outdated Show resolved Hide resolved
pkg/gui/presentation/files.go Outdated Show resolved Hide resolved
pkg/gui/presentation/files_test.go Outdated Show resolved Hide resolved
@stefanhaller stefanhaller added the enhancement New feature or request label Nov 30, 2024
@johannaschwarz
Copy link
Contributor Author

Thanks for your detailed input, I appreciate it!

My biggest issue is the commit history, which is a bit of a mess tbh. First of all, please don't merge master into a feature branch; rebase onto master instead. And then, I like commits that are self-contained and atomic, so that the chain of commits tells a logical story. In this case, there might be one commit that adds the model, config, and file loader, and then a second commit that makes use of it in the UI. But you might also squash everything into just one commit, I guess.

That's a valid point and I will do it this way in future commits. For this PR I will just squash it into a single commit.

@stefanhaller
Copy link
Collaborator

For this PR I will just squash it into a single commit.

Fine with me, please do.

@jesseduffield Do you also want to have another look at this? I'm very happy with the PR, I think it's ready to merge (with one tiny fixup still to be done).

One thing we didn't really decide yet is whether we keep the synchronous call to git diff --numstat. I would hope we can, since it makes things so much simpler. (And the code doesn't run when the option is disabled.)

Copy link
Owner

@jesseduffield jesseduffield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Happy with the extra synchronous calls given the only apply when the setting is disabled

When enabled, it adds "+n -m" after each file in the Files panel to show how
many lines were added and deleted, as with `git diff --numstat` on the command
line.
@stefanhaller stefanhaller merged commit 4a7cf60 into jesseduffield:master Dec 8, 2024
15 of 16 checks passed
mikavilpas added a commit to mikavilpas/dotfiles that referenced this pull request Dec 8, 2024
This is a great feature that was implemented in
jesseduffield/lazygit#4015

❤️
@johannaschwarz johannaschwarz deleted the line-changes branch December 12, 2024 13:33
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jan 28, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.44.1` -> `v0.45.2` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary>

### [`v0.45.2`](https://github.com/jesseduffield/lazygit/releases/tag/v0.45.2)

[Compare Source](jesseduffield/lazygit@v0.45.0...v0.45.2)

<!-- Release notes generated using configuration in .github/release.yml at v0.45.2 -->

#### What's Changed

##### Enhancements 🔥

-   Add ability to configure branch color patterns using regex by [@&#8203;mtrajano](https://github.com/mtrajano) in jesseduffield/lazygit#4130
-   Collapse/uncollapse all files in tree by [@&#8203;mtrajano](https://github.com/mtrajano) in jesseduffield/lazygit#4131
-   Improve undo action to restore files upon undoing a commit by [@&#8203;gabriellanata](https://github.com/gabriellanata) in jesseduffield/lazygit#4167

##### Fixes 🔧

-   Bump gocui (and tcell) by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4166

##### Maintenance ⚙️

-   Cut a new release automatically each month by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4146
-   Bump tcell to fix broken deployment by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4178

#### New Contributors

-   [@&#8203;mtrajano](https://github.com/mtrajano) made their first contribution in jesseduffield/lazygit#4130
-   [@&#8203;gabriellanata](https://github.com/gabriellanata) made their first contribution in jesseduffield/lazygit#4167

**Full Changelog**: jesseduffield/lazygit@v0.45.0...v0.45.2

### [`v0.45.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.45.0)

[Compare Source](jesseduffield/lazygit@v0.44.1...v0.45.0)

<!-- Release notes generated using configuration in .github/release.yml at v0.45.0 -->

Lots of great improvements in this one! Thanks to all the contributors who helped make this release happen.

Apologies for the long waits between releases. As of jesseduffield/lazygit#4146 we're now (almost) guaranteed to do monthly releases.

#### What's Changed

##### Enhancements 🔥

-   Better branch delete confirmation by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3915
-   Add a menu item to delete both local and remote branch at once by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3916
-   Auto-render hyperlinks by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3914
-   Allow pasting commits multiple times by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3983
-   Add config option to disable switching tabs with panel jump keys (and disable by default) by [@&#8203;hgreenstein](https://github.com/hgreenstein) in jesseduffield/lazygit#3927
    -   We jumped the gun on this change in the last release: having the number key both select a panel and switch tabs was bad for muscle memory so it's that behaviour is now disabled by default.
-   Add new filter to only show tracked files in Files panel by [@&#8203;veremenko-y](https://github.com/veremenko-y) in jesseduffield/lazygit#4024
-   Allow DiffContextSize to be decreased to zero by [@&#8203;LuxxxLucy](https://github.com/LuxxxLucy) in jesseduffield/lazygit#4050
-   Allow user to select remote and branch when creating a MR by [@&#8203;moha-gh](https://github.com/moha-gh) in jesseduffield/lazygit#1889
-   Delete range selection of branches by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4073
-   Show the number of lines changed per file in working file tree view by [@&#8203;johannaschwarz](https://github.com/johannaschwarz) in jesseduffield/lazygit#4015
-   Drop merge commits by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4094
-   Wrap lines in staging view by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4098
-   Allow to switch branches in Commit View ([#&#8203;4115](jesseduffield/lazygit#4115)) by [@&#8203;seflue](https://github.com/seflue) in jesseduffield/lazygit#4117
-   Allow on prem Azure DevOps Server pull request by [@&#8203;waynebowie99](https://github.com/waynebowie99) in jesseduffield/lazygit#3954
-   Add screen-mode command line argument by [@&#8203;alewis001](https://github.com/alewis001) in jesseduffield/lazygit#4103
-   Default to half-screen mode when opening with certain CLI args by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4141
-   Show the alt-enter keybinding at bottom of commit description view by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4136
-   Add number of commits to cherry-pick confirmation prompt by [@&#8203;brandondong](https://github.com/brandondong) in jesseduffield/lazygit#4158

##### Fixes 🔧

-   Fix copying commit author to clipboard by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3936
-   Fix merge conflict resolution when file doesn't end with a LF by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3976
-   Fix file icons by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3975
-   Fix non-sticky range select after clicking in staging view by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3998
-   Fix scroll position when entering staging view by clicking in the main view by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#3992
-   Fix mouse wheel scrolling of custom patch view by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4089
-   Improve editing a commit by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4090
-   Use custom context size in range diff by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4082
-   commits panel: add missing default sort order by [@&#8203;phanirithvij](https://github.com/phanirithvij) in jesseduffield/lazygit#4097
-   Fix issues with opening links and files using WSL by [@&#8203;bottino](https://github.com/bottino) in jesseduffield/lazygit#3850
-   Fix moving a commit across a branch boundary in a stack of branches by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4096
-   Don't preserve commit message when it's unchanged from initial message by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4110
-   Fix hang when returning from shell command by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4126
-   Prevent killing of unrelated processes on windows by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4137
-   Add worktree option to fast forwarding operation by [@&#8203;john-mutuma](https://github.com/john-mutuma) in jesseduffield/lazygit#4051
-   Don't show keybindings option in bottom line when panel is open by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4143
-   Fix reset/rebase to upstream by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4151
-   Avoid blank line at end of view by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4152
-   Adjust line number for working copy when editing a line by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4119
-   Fix `micro` editor preset by [@&#8203;kytta](https://github.com/kytta) in jesseduffield/lazygit#4156
-   Use interactive shell for running shell commands only if shell is bash or zsh by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4159

##### Maintenance ⚙️

-   Add performance improvements section to release notes by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#3922
-   ci: update `upload-artifact` and `download-artifact` actions to v4 by [@&#8203;Juneezee](https://github.com/Juneezee) in jesseduffield/lazygit#4063
-   Let schema/config.json end with a line feed by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4092
-   Saner view geometry by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4085
-   Cleanup background fetch by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4084
-   remove duplicate secondary MouseWheelUp keybind by [@&#8203;samueldominguez](https://github.com/samueldominguez) in jesseduffield/lazygit#4120
-   enhancement: Use HEX colors on file icons instead of C256 colors by [@&#8203;hasecilu](https://github.com/hasecilu) in jesseduffield/lazygit#4029
-   chore: use errors.New to replace fmt.Errorf with no parameters by [@&#8203;RiceChuan](https://github.com/RiceChuan) in jesseduffield/lazygit#4107
-   Fix typos by [@&#8203;NathanBaulch](https://github.com/NathanBaulch) in jesseduffield/lazygit#3920
-   Bump crypto and net packages by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4144
-   Tweak file icons by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4145
-   Standardise on 'screen mode' naming convention by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4142

##### Docs 📖

-   Fix installation for Ubuntu in README.md by [@&#8203;shaobosong](https://github.com/shaobosong) in jesseduffield/lazygit#4031
-   Fixes to lazygit Ubuntu install instructions in README.md by [@&#8203;wrecklass](https://github.com/wrecklass) in jesseduffield/lazygit#3970
-   Fix typos by [@&#8203;skoch13](https://github.com/skoch13) in jesseduffield/lazygit#4121
-   Add vision and design principles doc by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4053
-   docs: Stylize and correct the NixOS section in README.md by [@&#8203;wadsaek](https://github.com/wadsaek) in jesseduffield/lazygit#4135
-   Update chinese translation for pull requests by [@&#8203;jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4140
-   Set repology table to 3 columns by [@&#8203;hasecilu](https://github.com/hasecilu) in jesseduffield/lazygit#4138
-   Update instructions for using OSC52 with tmux by [@&#8203;EliasA5](https://github.com/EliasA5) in jesseduffield/lazygit#3597
-   Update docs to describe use of custom commands without keybindings by [@&#8203;a-jentleman](https://github.com/a-jentleman) in jesseduffield/lazygit#3846
-   documentation - describe os.openLink by [@&#8203;smangels](https://github.com/smangels) in jesseduffield/lazygit#3094
-   Clarify commit message prefix mechanics by [@&#8203;zeromask1337](https://github.com/zeromask1337) in jesseduffield/lazygit#4114

#### New Contributors

-   [@&#8203;shaobosong](https://github.com/shaobosong) made their first contribution in jesseduffield/lazygit#4031
-   [@&#8203;hgreenstein](https://github.com/hgreenstein) made their first contribution in jesseduffield/lazygit#3927
-   [@&#8203;wrecklass](https://github.com/wrecklass) made their first contribution in jesseduffield/lazygit#3970
-   [@&#8203;veremenko-y](https://github.com/veremenko-y) made their first contribution in jesseduffield/lazygit#4024
-   [@&#8203;LuxxxLucy](https://github.com/LuxxxLucy) made their first contribution in jesseduffield/lazygit#4050
-   [@&#8203;phanirithvij](https://github.com/phanirithvij) made their first contribution in jesseduffield/lazygit#4097
-   [@&#8203;bottino](https://github.com/bottino) made their first contribution in jesseduffield/lazygit#3850
-   [@&#8203;johannaschwarz](https://github.com/johannaschwarz) made their first contribution in jesseduffield/lazygit#4015
-   [@&#8203;samueldominguez](https://github.com/samueldominguez) made their first contribution in jesseduffield/lazygit#4120
-   [@&#8203;skoch13](https://github.com/skoch13) made their first contribution in jesseduffield/lazygit#4121
-   [@&#8203;wadsaek](https://github.com/wadsaek) made their first contribution in jesseduffield/lazygit#4135
-   [@&#8203;seflue](https://github.com/seflue) made their first contribution in jesseduffield/lazygit#4117
-   [@&#8203;waynebowie99](https://github.com/waynebowie99) made their first contribution in jesseduffield/lazygit#3954
-   [@&#8203;EliasA5](https://github.com/EliasA5) made their first contribution in jesseduffield/lazygit#3597
-   [@&#8203;alewis001](https://github.com/alewis001) made their first contribution in jesseduffield/lazygit#4103
-   [@&#8203;a-jentleman](https://github.com/a-jentleman) made their first contribution in jesseduffield/lazygit#3846
-   [@&#8203;john-mutuma](https://github.com/john-mutuma) made their first contribution in jesseduffield/lazygit#4051
-   [@&#8203;RiceChuan](https://github.com/RiceChuan) made their first contribution in jesseduffield/lazygit#4107
-   [@&#8203;NathanBaulch](https://github.com/NathanBaulch) made their first contribution in jesseduffield/lazygit#3920

**Full Changelog**: jesseduffield/lazygit@v0.44.1...v0.45.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xMzcuMSIsInVwZGF0ZWRJblZlciI6IjM5LjEzNy4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants