NeogitOrg / neogit

An interactive and powerful Git interface for Neovim, inspired by Magit
MIT License
3.83k stars 228 forks source link

Tracking Issue: Status buffer bugs on `nightly` branch #1233

Closed AThePeanut4 closed 3 months ago

AThePeanut4 commented 5 months ago

Firstly, thank you to everyone involved in making this plugin, and specifically thank you @CKolkey for all the fixes, new features and improvements you've made over the last year or so. It really makes working with Git a breeze, and without it I certainly would not have been able to fully switch over to Neovim.

Description

So this is a list of several small issues I've come across while (ab)using the rewritten status buffer. A few might also have existed before #1161. For brevity I'm just going to give short descriptions of each, if you're unable to reproduce any of them I'll be happy to provide more detailed steps.

Bugs:

Regressions (or perhaps intentional changes):

Reproduction steps for the <Tab> issue

  1. Open nvim in a clean clone of the neogit repo.
  2. Add a single unstaged change - just edit a file and save it.
  3. Open Neogit
  4. Move the cursor to the gap between the "Unstaged changes" section and the "Recent commits" section, and press <Tab>.
  5. Observe that the "Unstaged changes" section correctly collapses.
  6. Move the cursor to the "Unstaged changes" section, and press <Tab>.
  7. Observe that the "Unstaged changes" section correctly expands.
  8. Move the cursor to the line for the change you made ("Modified <filename>"), and press <Tab>.
  9. Observe that both the "Unstaged changes" section collapses and the "Recent commits" section expands.

Neovim version

NVIM v0.10.0-dev-2783+g381806729-Homebrew Build type: Release LuaJIT 2.1.1710088188

Operating system and version

macOS 14.4.1

CKolkey commented 5 months ago

Really appreciate the list. It's totally true than when you know how something is "supposed" to work, you just avoid doing the "wrong" things... 😅 Anyways, most of these are pretty straight forward, shouldn't be much work.

CKolkey commented 5 months ago

There aren't any arrows in the signcolumn anymore. It would be great to have these back, since they gave useful feedback especially to a new user as to what is expandable/collapsible.

When I updated things to use native folds, this kinda broke. Mostly because all diffs are lazy-loaded, so the fold doesn't exist (according to vim) until it's expanded for the first time.. I'm planning to bring it back, just haven't done it yet.

The "Head:", "Merge:" and "Push:" lines no longer include commit hashes. It would be nice to have these back as well, but if removing them was intentional then it's not a big deal. It just looks a bit weird without them now 😄

I never liked that, but I'm planning to make it an option you can configure. The new status buffer makes this pretty trivial, so I'll get to it soon :)

CKolkey commented 5 months ago

I didn't really "fix" the tab issue, but I overloaded j and k to "skip" the empty line between sections, so I think that kinda winds up solving the issue a different way.

AThePeanut4 commented 5 months ago

I didn't really "fix" the tab issue, but I overloaded j and k to "skip" the empty line between sections, so I think that kinda winds up solving the issue a different way.

Probably good enough for now, it was a fairly minor issue anyway. You can still get the cursor in the empty lines with the arrow keys, <C-U>, mouse, etc. I might try and figure out a proper fix at some point myself. I have found a small bug here though - when the Recent Commits section is collapsed, pressing j while the cursor is on the last line of the buffer (the Recent Commits header) now moves the cursor up by 1, rather than doing nothing.

I'm also unfortunately still seeing sections automatically close when returning from popups, although I think it only starts happening after I collapse a section from inside one the empty lines. I'm also seeing the cursor jump after returning from a popup - if the cursor was on the Hint/Head/Merge/Push lines, it jumps down to the Untracked files section.

CKolkey commented 5 months ago

Yeah, redrawing the buffer could be done a bit more cleverly, but I couldn't really crack it 😓

Internally it's a tree structure, the "root" has no parent node, but has child nodes, and those in-turn have child nodes, all the way down to the leaves, which are usually text of some kind, which have no children. I tried a few recursive approaches to diff the trees, tried some morphing algo's from the javascript world, and... never got anything to work. So, the current approach is to just render the new tree, and copy over just the fold state of foldable nodes.

Which is... close to right, but requires some other complexity to make up for it.

AThePeanut4 commented 5 months ago

https://github.com/NeogitOrg/neogit/issues/1236#issuecomment-2046121658 @avegancafe

@CKolkey I am also getting what is probably the same issue: image This happens with my normal config and when using a clean config (nothing but neogit).

Uncommenting the fillchars line here, which was commented out in 8e489a04, fixes it: https://github.com/NeogitOrg/neogit/blob/abbb35a48f47ebd786ef44174b080c61e7fb7cf0/lua/neogit/lib/buffer.lua#L704-L714

Should the lines that set signcolumn not also be uncommented? If I uncomment them then the signcolumn decreases in width by 1, so that there is no longer a gap after the >, but this is only because I set signcolumn=yes in my config. In a clean config nothing changes (there is no gap either way). On that topic though, I have to say I don't like the removal of the gap after the > (18938e2). I could just change the signs to add a space in my neogit config, but for consistency with before I think it would be a good idea to either revert that part of 18938e2 or change the default signs to include a space.

CKolkey commented 5 months ago

Great catch - I was messing with something else and accidentally committed that change... Anyways, its back 😅

As for the space there - I was totally on the fence about that one -- I'll put the space back, I like it too. Thanks for the feedback.

avegancafe commented 5 months ago

Yeah probably pretty similar @AThePeanut4 , I'm still seeing this on latest nightly, so lmk whenever I should pull and can test it out!