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

Neogit doesn't use the user's configured .gitmessages file #66

Closed
akinsho opened this issue Mar 22, 2021 · 14 comments · Fixed by #155
Closed

Neogit doesn't use the user's configured .gitmessages file #66

akinsho opened this issue Mar 22, 2021 · 14 comments · Fixed by #155
Labels
bug Something isn't working

Comments

@akinsho
Copy link
Contributor

akinsho commented Mar 22, 2021

image

Just to flag this although it might be a known issue already but whilst using neogit it doesn't read my .gitmessages from my .gitconfig. I've set the commit message text to use a different placeholder to match the commit style I use. The screen shot show the commit buffer with vim fugitive and neogit. Fugitive somehow is reading this information whereas neogit doesn't.

It's not a blocking issue at all just worth pointing out.

@TimUntersberger
Copy link
Collaborator

We are currently manually rebuilding the commit content and we are not respecting any configiurations on the git side.

Are you using any other configurations on the git side?

@akinsho
Copy link
Contributor Author

akinsho commented Mar 22, 2021

I have git config files for work and personal use that make some changes to the defaults for example

[push]
  followTags = true
[pull]
  rebase = true
[rebase]
  autoStash = true

These are some things I've tweaked that aren't recognised by neogit .

EDIT: I forgot to include the one that's actually relevant to this issue

[commit]
  template = ~/.gitmessage

@TimUntersberger
Copy link
Collaborator

@RianFuro do you have any opinions regarding respecting specific git configurations?

Some things like gitmessage template are a must have imo, but should we also support things like push followTags and whatever (Just like he has configured)? I never changed git defaults so I don't even know what is possible and what each settings does.

@RianFuro
Copy link
Contributor

RianFuro commented Mar 22, 2021

Yeah same here. I have gpg signing configured so I made sure that still worked when committing, but I must admit I use less of git than I probably should.

On a conceptual level I feel we should support everything at some point; otherwise we'd betray user expectations when they decide to give neogit a try.
That being said, since we just call the cli like a normal user would in most cases, things should "just work". For example, I'm interested in why pulling via neogit would ignore the rebase = true setting; maybe the --no-commit arg we add is forcing a merge or something?

Ideally we'd really only have to take care where we deviate from calling git "normally", e.g. our commit workflow, and minimize those places as much as possible.

@TimUntersberger
Copy link
Collaborator

Ideally we'd really only have to take care where we deviate from calling git "normally", e.g. our commit workflow, and minimize those places as much as possible.

👍

@TimUntersberger TimUntersberger added the bug Something isn't working label Mar 23, 2021
@Odie
Copy link

Odie commented Mar 24, 2021

Might want to try setting the HOME env var when launching git. It should help it locate the user’s .gitconfig files.

@RianFuro
Copy link
Contributor

@Odie we already do that:
https://github.com/TimUntersberger/neogit/blob/537cc6e1757c41bd75717ebd4421c27b7ebe9205/lua/neogit/process.lua#L25-L38
(and afaik not setting env at all makes spawn use the parent's environment)

@Odie
Copy link

Odie commented Mar 24, 2021

Okay. I played around with the code a bit. Here's what I found:

  1. param.env is not usually set
    As neovim starts up, without even going to the status buffer, neogit fires off about 10 calls to vim.loop.spawn() via neogot/process.lua. Going into the status buffer via ":Neogit" will fire off 1 more. Of these 11 calls, 8 of them had
    options.env = {}. The remaining 3 of them did not include the options.env field at all.

    This means that the above code @RianFuro referenced, which adds HOME and GNUPGPHOME, never fires, at least for these 11 calls that I saw.

  2. It's unclear if env gets inherited
    http://docs.libuv.org/en/v1.x/guide/processes.html says this:

    uv_process_options_t.env is a null-terminated array of strings, each of the form VAR=VALUE used to set up the environment variables for the process. Set this to NULL to inherit the environment from the parent (this) process.

    I'm not sure if setting params.env explicitly to nil will cause this to happen. But in any case, params.env is never set to nil, so env inheritance probably won't automatically happen.

  3. Bringing up the commit buffer does not go through spawn or git
    No calls were made to git using vim.loop.spawn() in neogit/process.lua. So even if $HOME is successfully set, it would not affect how the contents of the commit buffer is prepped.


I'm not familiar with the codebase enough try putting in fixes here. This particular issue can probably be fixed along with #54, by getting git to prep COMMIT_EDITMSG directly.

@RianFuro
Copy link
Contributor

RianFuro commented Mar 24, 2021

I'm not sure if setting params.env explicitly to nil will cause this to happen. But in any case, params.env is never set to nil, so env inheritance probably won't automatically happen.

It's not that involved really. You don't really have to possibility in lua to distinguish between a field set to nil and a field not set at all; the 2 can be thought of as equivalent. Actually, not even the c-api can distinguish the 2: calling lua_getfield with a key that does not exist on the table still just pushes nil onto the stack.
So yes, not setting env should have the child automatically inherit the parent process' environment.

You can "easily" check that by constructing a toy example (template from here, just adapted the called script):

local uv = vim.loop
local stdin = uv.new_pipe(false)
local stdout = uv.new_pipe(false)
local stderr = uv.new_pipe(false)

print("stdin", stdin)
print("stdout", stdout)
print("stderr", stderr)

local handle, pid = uv.spawn("sh", {
  args = {'test.sh'},
  stdio = {stdin, stdout, stderr}
}, function(code, signal) -- on exit
  print("exit code", code)
  print("exit signal", signal)
end)

print("process opened", handle, pid)

uv.read_start(stdout, function(err, data)
  assert(not err, err)
  if data then
    print("stdout chunk", stdout, data)
  else
    print("stdout end", stdout)
  end
end)

uv.read_start(stderr, function(err, data)
  assert(not err, err)
  if data then
    print("stderr chunk", stderr, data)
  else
    print("stderr end", stderr)
  end
end)

uv.write(stdin, "Hello World")

uv.shutdown(stdin, function()
  print("stdin shutdown", stdin)
  uv.close(handle, function()
    print("process closed", handle, pid)
  end)
end)

Is sadly the simplest way to call spawn.
test.sh is then simply:

#!/bin/sh
echo $MY_VAR

Executing the above script from neovim (for example with luafile %), passes the environment var down to the script as expected (if set when spawning nvim of course)

@Odie
Copy link

Odie commented Mar 24, 2021

I see! Thanks for that code snippet. I tried dumping $HOME both via a shell script and yet another lua script. Both printed out $HOME successfully. x)

So does that mean if neogit just asked git to prep COMMIT_EDITMSG as usual, everything should just work?

@Iron-E
Copy link

Iron-E commented Apr 19, 2021

I'll add this comment here (since I believe this may be related to the parent issue), but sometimes the message showing which files have been staged for commit doesn't update to reflect which files are actually going to be committed. I don't have a repro for this, but I have noticed in using Neogit over the past few days that if you mix committing using the CLI and with Neogit it will not update the status message.

For example, it will say that I am committing file Foo.rs (which was committed previously) when I am actually committing Bar.rs. It will also the old list of unstaged files from the previous commit, as well. Doesn't seem to consistently happen, but it does once in a while.

I think fixing whatever issue this is might also fix that one, since it's all touching similar code (if I have read this issue correctly).

@akinsho
Copy link
Contributor Author

akinsho commented Jul 1, 2021

So I had a look through the mountain of vimscript in vim-fugitive for some hints about how it has managed to solve this. I'm no expert in vimscript but it seems that the config file is being read all over the place and used to get the value of some options.

I'm not sure this is the best solution/why it was needed since a git config also allows a user to do things like [include] paths to other files to be read and a user can also do this conditionally using [includeIf] which means you'd have to resolve these as well.

@TimUntersberger
Copy link
Collaborator

I'm not sure this is the best solution/why it was needed since a git config also allows a user to do things like [include] paths to other files to be read and a user can also do this conditionally using [includeIf] which means you'd have to resolve these as well.

Maybe there is an easy way to do this using git config.

Maybe git config --global --get commit.template ?

@Iron-E
Copy link

Iron-E commented Jul 8, 2021

I made a demo for this. Currently only the 'c' action is supported. Not sure if I will make a full PR or not, depends on how time-consuming the rest of the commit actions end up being :P

I notice the reword and amend git texts look a bit different (they mention author name, commit date, etc). Would probably have to parse git --no-pager log output for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants