isakbm / gitgraph.nvim

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

check if git repo exists before creating buffer #13

Closed iguanacucumber closed 3 months ago

iguanacucumber commented 3 months ago

Instead of this, why don't we do

1. check that `git` is installed

Checking if git is installed is a bit useless, since to download this plugin you need to use git

2. check that git status has zero exit code indicating success and that this is a git repo :)

I tried to do that, without sucess ( i don't know alot about lua tbh )

isakbm commented 3 months ago

Alright, I'll paste a code snippet or two in a bit :)

isakbm commented 3 months ago

Took a bit of tinkering but this should do the trick, start by declaring this function

---@param cmd string
---@return boolean -- true if failure (exit code ~= 0) false otherwise (exit code == 0)
--- note that this method was sadly neede since there's some strange bug with lua's handle:close?
--- it doesn't get the exit code correctly by itself?
function check_cmd(cmd)
  local res = io.popen(cmd .. '; echo $?')
  if not res then
    return true
  end
  local last_line = '1'
  for line in res:lines() do
    last_line = line
  end
  res:close()
  return last_line ~= '0'
end

Then call it like so as first thing inside draw

  if check_cmd('git --version') then
    print('git command not found, please install it')
    return
  end

  if check_cmd('git status') then
    print('does not seem to be a valid git repo')
    return
  end

note that there is no need for an else wrapping the rest of the draw thanks to early return. Nice to avoid that unnecessary indentation.

isakbm commented 3 months ago

I think I'll just make a commit on the main branch that resolves this new issue #14

Thanks for inspiration though, and please prefer this upstream change once I get it in :)

iguanacucumber commented 3 months ago
if check_cmd('git --version > /dev/null 2>&1') then
    vim.notify('git command not found, please install it', vim.log.levels.ERROR)
    return
  end

if check_cmd('git status > /dev/null 2>&1') then
  vim.notify('does not seem to be a valid git repo', vim.log.levels.ERROR)
  return
end
isakbm commented 3 months ago

The > /dev/null pipe or redirect or whatever its called is a bit unnecessary, it is silent since it is already piped to the file that is used to read the results into by lua. 🤔

iguanacucumber commented 3 months ago

The > /dev/null pipe is a bit unnecessary, it is silent since it is already piped to the file that is used to read the results into by lua. 🤔

maybe neovim updated this line at the same that this function was run so you didnt see it but: 20240810_23h57m22s_grim

isakbm commented 3 months ago

huh, I got it now too, I've never seen this before 👀

image

iguanacucumber commented 3 months ago

you we're kinda right, we don't need git > /dev/null we just need git 2>&1

isakbm commented 3 months ago

ah yeah that makes sense I guess, I think io.popen only consumes the stdout, and not the stderr ?

iguanacucumber commented 3 months ago

yep it seems that io.popen only consumes stdout so we have to redirect stderr to stdout with 2>&1

isakbm commented 3 months ago

Thoughts ?

local log = {}

function log.info(msg)
  vim.notify(msg, vim.log.levels.INFO)
end

function log.error(msg)
  vim.notify(msg, vim.log.levels.ERROR)
end

return log

and

---@param cmd string
---@return boolean -- true if failure (exit code ~= 0) false otherwise (exit code == 0)
--- note that this method was sadly neede since there's some strange bug with lua's handle:close?
--- it doesn't get the exit code correctly by itself?
function check_cmd(cmd)
  local res = io.popen(cmd .. ' 2>&1; echo $?')
  if not res then
    return true
  end
  local last_line = '1'
  for line in res:lines() do
    last_line = line
  end
  res:close()
  return last_line ~= '0'
end

---@param options I.DrawOptions
---@param args I.GitLogArgs
function M.draw(options, args)
  if check_cmd('git --version') then
    log.error('git command not found, please install it')
    return
  end

  if check_cmd('git status') then
    log.error('does not seem to be a valid git repo')
    return
  end
iguanacucumber commented 3 months ago

so if you need use this function again we can do something like this to avoid adding 2>&1 every check_cmd function call:

  function check_cmd(cmd)
    cmd = cmd .. " 2>&1" -- redirects stderr to stdout since io.popen uses stdout
    local res = io.popen(cmd .. '; echo $?')
    if not res then
      return true
    end
    local last_line = '1'
    for line in res:lines() do
      last_line = line
    end
    res:close()
    return last_line ~= '0'
  end

  if check_cmd('git --version') then
    vim.notify('git command not found, please install it', vim.log.levels.ERROR)
    return
  end

  if check_cmd('git status') then
    vim.notify('does not seem to be a valid git repo', vim.log.levels.ERROR)
    return
  end
iguanacucumber commented 3 months ago

Thoughts ?

local log = {}

function log.info(msg)
  vim.notify(msg, vim.log.levels.INFO)
end

function log.error(msg)
  vim.notify(msg, vim.log.levels.ERROR)
end

return log

and

---@param cmd string
---@return boolean -- true if failure (exit code ~= 0) false otherwise (exit code == 0)
--- note that this method was sadly neede since there's some strange bug with lua's handle:close?
--- it doesn't get the exit code correctly by itself?
function check_cmd(cmd)
  local res = io.popen(cmd .. ' 2>&1; echo $?')
  if not res then
    return true
  end
  local last_line = '1'
  for line in res:lines() do
    last_line = line
  end
  res:close()
  return last_line ~= '0'
end

---@param options I.DrawOptions
---@param args I.GitLogArgs
function M.draw(options, args)
  if check_cmd('git --version') then
    log.error('git command not found, please install it')
    return
  end

  if check_cmd('git status') then
    log.error('does not seem to be a valid git repo')
    return
  end

i think this is perfect

isakbm commented 3 months ago

seems we had a race condition :D

iguanacucumber commented 3 months ago

i think this should work:

local function check_cmd(cmd)
  local result = vim.fn.system(cmd)
  local exit_code = vim.v.shell_error

  return exit_code ~= 0
end
isakbm commented 3 months ago

Truly, thank you! 🎉

image

isakbm commented 3 months ago

i think this should work:

local function check_cmd(cmd)
  local result = vim.fn.system(cmd)
  local exit_code = vim.v.shell_error

  return exit_code ~= 0
end

Feel free to make PR for that subtle change if you want, should be much quicker to approve now! 🎉

isakbm commented 3 months ago

... but again we would need to check that it does not pollute some buffer with the stdout / stderr ... 😬

isakbm commented 3 months ago

I'll close this pull request now if you don't mind :)