search

projekt0n / github-nvim-theme

Github's Neovim themes
MIT License
2.02k stars 105 forks source link

fix(compiler): broken `.git` dir path resolution #269

Open tmillr opened 1 year ago

tmillr commented 1 year ago

Currently, in the file lua/github-theme/init.lua, the code util.join_paths(debug.getinfo(1).source:sub(2, -23), '.git') returns an absolute path ending in **/lua/.git which is not, and will never be, a valid path to the .git directory. This means that recompilation does not currently occur when the plugin updates or changes (unless user config changes too) and that users may miss crucial updates (e.g. bug fixes).

  1. Fix bug by changing util.join_paths(debug.getinfo(1).source:sub(2, -23), '.git') to debug.getinfo(1).source .. '/../../../.git', and then use luv's/libuv's fs_stat() to get the last modified time of this path. This change does not rely on any particular filenames existing in the path, but it still relies on a hard-coded depth. If the path does not exist or the stat is unsuccessful, force recompilation (as otherwise plugin updates could be missed by many users).

  2. Use libuv/luv fs_stat() to get .git dir's mtime with nanosecond precision (NOTE: this function is only available by default in Neovim, not Vim, however, it appears that this plugin isn't compatible with Vim currently anyway, so this shouldn't be an issue).

  3. Correctly handle .git files, as .git is not always a directory.

See #262

nullchilly commented 1 year ago
  1. This change doesn't work (See the video below) and what is the purpose of uv.sleep(1)?

https://github.com/projekt0n/github-nvim-theme/assets/56817415/1a606fda-2147-4826-b9e6-3bc302a57dfd

  1. vim.loop.fs_stat is 2 times slower than vim.fn.getftime, no reason to use it.

  2. It was already correctly handled, git path was used instead of mtime for nixos.

tmillr commented 1 year ago
  1. This change doesn't work (See the video below) and what is the purpose of uv.sleep(1)?

Yes, it doesn't appear to be working, there is a bug somewhere. uv.sleep(1) is being used as a backoff timer for handling/retrying on such errors as EINTR and EAGAIN, although it could probably be optimized a bit further. Luv does not handle these type of errors for you automatically AFAIK.

  1. vim.loop.fs_stat is 2 times slower than vim.fn.getftime, no reason to use it.

Ok. To be fair, we're talking about a difference of +0.02ms for each call for just a couple of calls, and many nvim plugins utilize libuv. But I guess the vim api is easier to use as well.

  1. It was already correctly handled, git path was used instead of mtime for nixos.

How? Does git touch .git files every time a worktree's HEAD moves or something? That would seem strange. I'm talking about literal .git files (such as those used in git worktrees), not .git dirs. And what do you mean by git path was used instead of mtime for nixos?

tmillr commented 9 months ago

I should probably close this, this has all been fixed for the most part.

.git files and git worktrees are still not handled properly I believe, but this is somewhat of an edge-case. Also it might be worth investigating or understanding the behavior when plugin dir is not in a git repo nor nix store?