isakbm / gitgraph.nvim

Git Graph plugin for neovim
MIT License
309 stars 7 forks source link

git help checker is incorrect for Windows; always get error "git command not found, please install it" #21

Closed GitMurf closed 3 months ago

GitMurf commented 3 months ago

I think the help utility checker (see here) for git install is incorrect for Windows. The graph never even tries to load because it gets blocked by the git checker with: "git command not found, please install it"

This is the result I get when running the

"git version 2.45.2.windows.1"

The checker seems to expect multiple lines and makes sure the last one is not equal to 0 but it appears on Windows it all just comes back as a single string.

Or am I mis-understanding something on how this is working?

isakbm commented 3 months ago

That seems very plausible.

Please try to find a solution and make a PR. I will not be able to resolve this myself as I use WSL and therefore I don't run vim inside windows directly.

I don't even know how to get vim to work correctly and performantly on windows. Every time I tried it was both easier and faster to get it running inside WSL.

brunobmello25 commented 3 months ago

hey, I believe I managed to fix this issue with this PR

@GitMurf could you test it in your machine as well and see if it's working as intended? just clone the fork and adjust your plugin manager to pull gitgraph.nvim from your cloned fork. If you are using lazy, you can do this like this:

  {
    dir = '...',
    dependencies = {
      'sindrets/diffview.nvim',
    },
    config = function()
      -- ...
    end,
  }
GitMurf commented 3 months ago

could you test it in your machine as well and see if it's working as intended? just clone the fork and adjust your plugin manager to pull gitgraph.nvim from your cloned fork.

@brunobmello25 i assume you mean use your fork here? https://github.com/brunobmello25/gitgraph.nvim

Instead of cloning, can't I just point lazy plugin spec to your repo?

brunobmello25 commented 3 months ago

could you test it in your machine as well and see if it's working as intended? just clone the fork and adjust your plugin manager to pull gitgraph.nvim from your cloned fork.

@brunobmello25 i assume you mean use your fork here? https://github.com/brunobmello25/gitgraph.nvim

Instead of cloning, can't I just point lazy plugin spec to your repo?

You are right! You can do that to 😅

GitMurf commented 3 months ago

@brunobmello25 your PR fixes the git check cmd util function. nice work. But there is one more fix needed for the plugin to work on windows (cmd). Instead of me doing a separate PR, do you mind just adding this fix as well? all you need to do is update this line here: https://github.com/brunobmello25/gitgraph.nvim/blob/9c0fb9290b195d9cc60e9257b193268871b88d8d/lua/gitgraph/git.lua#L25

To this:

  local cli = [[git log %s %s --pretty="%s" --date="%s" %s %s --date-order]]

The change is simple... just needs " instead of ' for quotes. I am not sure if you want to add a is_windows condition on this or if it works the same in unix shells (I cannot test).

Let me know your thoughts and if you have any questions?

brunobmello25 commented 3 months ago

@GitMurf thanks for testing and helping with the PR! I've just pushed the fix. could you test again? should be working now.

I tested the change on WSL2 and it also works fine with double quotes, so I don't think we need a is_windows check on this part

GitMurf commented 3 months ago

@brunobmello25 confirmed working! nice work and thanks!

brunobmello25 commented 3 months ago

@GitMurf that's great to hear! @isakbm I believe we are good to go 😅 let me know if you feel like you need something to change. Otherwise feel free to merge

GitMurf commented 3 months ago

@isakbm and @brunobmello25 just confirming that after the merged PR and updating the plugin everything is working as expected on Windows!