NeogitOrg / neogit

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

Some issues (and feature requests) for the status buffer/window #1258

Open petobens opened 3 months ago

petobens commented 3 months ago

Hi! I'm trying to move away from fugitive into neogit (kudos for all the work). I have encountered a few issues/bugs in the Status window as it can be seen in my first image below ( the second one is from fugitive):

  1. When I enable line numbers the actual line numbers are mangled (i.e they don't increase sequentially by 1 but go from 13 to 22, etc. Also relative numbers don't seem to be enabled.)
  2. I use a statuscol plugin to set my statuscol. In particular I have fold signs appearing before line numbers. Neogit doesn't seem to respect that setting.
  3. Is it possible to add custom highligts for Head:, Merge:, commit hash and count (number between ())?. Also I don't quite get the big padding after Head:, can it be shortened?
  4. There is an extra empty line at the end of the buffer: line 73.
  5. I like the status window to open below every other window and with a specific height. To that end I use:
    vim.keymap.set('n', '<Leader>ng', function()
    neogit.open()
    vim.cmd('wincmd J | resize 15 | set winfixheight')
    end)

    I was wondering whether the kind argument could receive a function instead of a string to do this directly from the config instead of opening and then applying the layout in the mapping.

    1. If I press q to close the status buffer the cursor doens't necessarily go the last window id.

Thanks in advance!

FWIW my current config is here: https://github.com/petobens/dotfiles/blob/master/nvim/lua/plugin-config/neogit_config.lua

image image

CKolkey commented 3 months ago

Hey! Good notes - lets do it.

  1. The line numbers are right - it's because there are folded sections of the buffer. This would be the case for any buffer in vim with folds.. nothing I can do about that. For what it's worth, I don't use the numbers in any neogit buffer.. don't see a reason to 😅

  2. Right now it's set up to ignore the users statuscolumn setting and set it ourself. The alternative behaviour would be to just not set anything if the user has something custom, but then the fold icons wouldn't work as expected, since some folds (the diffs) are lazy loaded and are not "real" folds until the user opens them. If I ignore the user setting, then I can make it "correct" in that respect.

  3. ~Yeah, it might already be the case - I have to check - but otherwise that's super simple. I can make the padding configurable, too.~ Added: https://github.com/NeogitOrg/neogit/pull/978/commits/c31acc55404a04dca04742049eadf0895c9c5444 https://github.com/NeogitOrg/neogit/pull/978/commits/3c83d1b8cd4397eb230e810583d117cfec45b3cd

  4. I know. Getting rid of it is more complex than I'd like... basically, when rendering sections, the last section doesn't "know" it's the last section.. and all sections have a blank line at the end to pad them from the section beneath them, but only when unfolded. I'll deal with this at some point, but it basically means a post-processing step where I have to recalculate the location of anything that includes the last line... someday. And, for what its worth, Magit has a blank line there too 😉

  5. 'Kind' cant take a function, as it gets passed into the buffer constructor - and eventually into the neovim api. What you -could- do for this kind of behaviour is add an ftplugin file for NeogitStatusBuffer files.

  6. Yeah, this one is surprisingly hard to get right. I use neogit as a tabpage, but i'll try it as a split and see if that works as expected or what

petobens commented 3 months ago

Thanks for the quick reply!

  1. I think there is something else going on with linenumbers. In the GIF below I simply have 2 lines (line 1, line 2 as you can see in my statusline) and line numbers are still mixed up saying "10, "20" (even when disabling folds) neogit_ln

  2. Personally I prefer to have my own statuscolumn behaviour. Maybe that could be put behind a config flag?

  3. Great! Thanks for adding that. Would be possible to add highlight for the file count? While playing with these highlights I also noticed that i) show_head_commit_hash is not documented, ii) would it be possible to add a similar setting to avoid showing the commit message? (I only want to see master and origin branches and iii) should the show_head_commit_hash be inside the status config section? I general I noticed that several settings that affect the status section such as disable_hints, disable_line_numbers, signs etc are at a global level rather than within the status subsection.

  4. Ok

  5. I tried adding the following:

    kind = function()
        vim.cmd('wincmd J | resize 15 | set winfixheight')
    end,

    but then I get:

    
    Failed to run `config` for neogit

====Neogit Configuration Errors==== Neogit has NOT been setup! You have a misconfiguration in your Neogit setup! Validate that your configuration passed to require("neogit").setup() is valid!

Config value: kind had error -> Expected kind to be of type 'string', got 'function'

stacktrace:

  1. Thanks!
CKolkey commented 3 months ago
  1. Ok, yeah, that's a bit wack. Tell you what - I rewrote the status column stuff last night to just set a signcolumn sign for the folds (I know, if you show the foldcolumn too, it'll look strange, but I couldn't set different signs for different types of folds..). But, that means it won't mess with line numbers 🤷🏼

  2. Fair - it's gone now.

  3. Yes :) i. Didn't realize I forgot to document the commit hash one - done now. ii. Yes, but I'm not sure which part you mean? iii. Yes, good call. iv. The config is (slightly) unorganized... A lot of it is a few years old. I try really hard to not break it, so people don't need to update their setup.. One of life's compromises.

  4. (github auto numbers things...)

  5. Yeah, that wouldn't work - needs to be a string. What I meant was that if you add a file like so:

    
    -- ~/.config/nvim/after/ftplugin/NeogitStatusBuffer.lua

vim.cmd('wincmd J | resize 15 | set winfixheight')

petobens commented 3 months ago

1,2,4,5. Great!

Regarding 3. iii: I meant whether it is possible to add a setting to hide not only the commit_hash but also the commit message; in the image below I would like to hide the vim: more diffview mappings and higlighting bit .

image

petobens commented 3 months ago

A couple of extra comments (again by comparing fugitive with neogit).

  1. Is there a reason to have Untracked files or Unstaged changes / Staged changes instead of simply Untracked, Unstaged, Staged (i.e isn't the file or changes redundant within this context?)
  2. Can the NeogitChangeM highlight group depend on whether the symbol is inside a unstaged/staged block? Otherwise, as in the image, I have the M staged icon highlighted in red (whereas I would like it to be in green)

image

CKolkey commented 3 months ago

Yeah, check out how magit's status buffer looks:

Screenshot 2024-04-24 at 23 47 48

Fugitive's status buffer is also inspired from magit: https://github.com/tpope/vim-fugitive/issues/569

I don't particularly think the phrasing "untracked files, unstaged changes, etc" is overly verbose. I know that's a matter of opinion/taste, but I'm fine with it as-is.

To your second point, it would be pretty easy to make the highlight group of each section be unique, and link back to the original. I'll see what I can do about that.

petobens commented 3 months ago

To your second point, it would be pretty easy to make the highlight group of each section be unique, and link back to the original. I'll see what I can do about that.

Awesome thanks

On a side note, I was thinking about what you said here:

iv. The config is (slightly) unorganized... A lot of it is a few years old. I try really hard to not break it, so people don't need to update their setup.. One of life's compromises.

and as per the whole statuscolumn cleanup I guess that now there is no need for a signs = {hunk, item, section} config right? Also disable_signs should always be set to true? Otherwise there is unneeded extra space in the sign gutter as it can be seen in the GIF:

Peek 2024-04-25 20-48