neogit fails to load in some repos

[amtoine]

Description

when i open Neogit inside the Nushell repo, i get an error > Note

see actual behaviour section below

at first, i thought it could be in bigger repos, like Nushell's, but then i tried with the source base of Neovim itself and Neogit opened :thinking:

Neovim version

NVIM v0.9.5 Build type: Release LuaJIT 2.1.1702233742

Operating system and version

Linux archlinux 6.7.9-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 08 Mar 2024 01:59:01 +0000 x86_64 GNU/Linux

Steps to reproduce

1. git clone https://github.com/nushell/nushell
2. cd nushell
3. nvim -nu minimal.lua
4. run :Neogit

Important it appears cloning the repo from scratch solves the issue :thinking: so there must be something in my local copy that breaks Neogit, not sure what...

Expected behavior

i expected the status to open

Actual behavior

i get the following error and a blank status buffer

...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:18: The coroutine failed with this message: ...ne/.local/share/nvim/lazy/neogit/lua/neogit/lib/json.lua:63: Failed to parse log json!: Expected value but found invalid token at character 183402
t\":\"dmy","wgMonthNames":["",<\>n"January","February","March",
stack traceback:
^I[C]: in function 'error'
^I...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:18: in function 'callback_or_next'
^I...share/nvim/lazy/plenary.nvim/lua/plenary/async/async.lua:45: in function 'cb'
^I...ine/.local/share/nvim/lazy/neogit/lua/neogit/process.lua:359: in function <...ine/.local/share/nvim/lazy/neogit/lua/neogit/process.lua:325>

Note i used :messages to see more than one line, if there's a better way to do that, please let me know :relieved:

Minimal config

vim.g.mapleader = ' '
vim.g.maplocalleader = ' '

-- install Lazy
local lazypath = vim.fn.stdpath 'data' .. '/lazy/lazy.nvim'
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system {
    'git',
    'clone',
    '--filter=blob:none',
    'https://github.com/folke/lazy.nvim.git',
    '--branch=stable', -- latest stable release
    lazypath,
  }
end
vim.opt.rtp:prepend(lazypath)

-- install Neogit
require('lazy').setup({
  {
    "NeogitOrg/neogit",
    dependencies = { "nvim-lua/plenary.nvim", },
    config = function()
      require('neogit').setup({})
    end
  }
})

[CKolkey]

I rewrote all the log parsing, so this should be fixed.

[amtoine]

@CKolkey ohh wow, amazing, thanks :pray:

i just hit "update" in Lazy and it is fixed, noice :muscle:

may i ask you what the issue was and where it was fixed in this repo, i'm just curious :wink: :innocent:

[CKolkey]

Totally! I had this brilliant idea about how to parse the git log cli output - it lets you pass a format argument, which gets used in a printf() thing in git. My genius idea was to format it as -json-, and then use nvim's json encode/decode functions to manage things. This totally worked, but was vulnerable to something like a sql-injection attack: if a commit message had a " character, thats valid json, so... had to start escaping fields. That also worked, but... it was brittle.

So, new solution was to just use \29, \30, and \31 - field/record/group delimiting control characters, and split on those. You can check lib/json.lua on master vs lib/record.lua for the old and new implementations. I can post links later when I'm at my computer 😊

[amtoine]

ooooh, "log parsing" meant "parsing the output of git log", that makes so much more sense now!!

thanks for the hints on what was happening :pray: collapsing everything down to a common format, e.g. JSON, sounds like a good idea from the beginning, especially if you're able to make it more robust over time, pretty cool :star_struck:

i'm using Nushell as my daily driver and i love the idea of converting everything to a common format, in my case Nushell values, and then working with a set of tools i know :muscle:

links would be amazing, but otherwise, thanks again for the answers, really nice interaction there :heart:

[CKolkey]

Sure :) So, here's the version that was on master for a while: https://github.com/NeogitOrg/neogit/blob/10feeeca0d8833756e0ae4c175aee8e89ed4508b/lua/neogit/lib/json.lua

M.encode() takes a table and turns it into a json string, and M.decode() takes the output of git log (with the --format flag we made) and turns it into a lua table. Here's the caller: https://github.com/NeogitOrg/neogit/blob/10feeeca0d8833756e0ae4c175aee8e89ed4508b/lua/neogit/lib/git/log.lua#L370-L371

It has to specify which fields should be escaped, which kinda sucks, and it only works most of the time.

So, enter the next draft (that I forgot about since it only lasted a day or two): https://github.com/NeogitOrg/neogit/blob/c27fcb2a96d54cb32f1ca780f2d7398f73fb3ba3/lua/neogit/lib/json.lua

The idea there was to take advantage of lua's weird [=[ ]=] string delimiters. Basically, hope that the closing string delimiter isn't in a git commit, and then eval() the cli output into a lua table. Technically worked fine, but... not the fastest, and, required evaluating input as code... which... felt like it might be a dumb idea.

So, last idea: https://github.com/NeogitOrg/neogit/blob/master/lua/neogit/lib/record.lua

Uses ASCII escape characters (group separator, record separator, unit separator) to delimit the git log output, so it's easy and reliable to split between keys/values, pairs of keys/values, and collections of pairs of keys/values.

This would fail if someone added the control characters to their git commit message, but.. I think that's on them for adding invisible control characters to their commit message.

One of the root issues that makes this problem tough is that \n is perfectly valid in git commit messages, but is tougher to parse.