plugins/oil-git-status: init

[FKouhai]

this PR adds oil-git-status to nixvim, since it recently got into master branch of nixpkgs
https://github.com/refractalize/oil-git-status.nvim

[FKouhai]

finally I made it work

[FKouhai]

pushed with a comment on how to successfully lazy load this plugin

[MattSturgeon]

Thanks for your persistence, looks like this is nearly there 🚀

[FKouhai]

Thanks for your persistence, looks like this is nearly there 🚀

thank you :) you've been giving me really good reviews I really appreciate them

[MattSturgeon]

Otherwise LGTM

[MattSturgeon]

This is kinda subjective, but if we interpolate the actual option, users will see either plugins.oil.enable or programs.nixvim.plugins.oil.enable depending on whether their nixvim configuration is a programs.nixvim submodule.

This also has the advantage of ensuring no typos.

Suggested change

	plugins.oil.enable = true;
	${options.plugins.oil.enable} = true;

To do this, you need the options module arg of course.

Also, nit: I would indent the code example and add a colon to the end of the previous line, to be consistent with the other warning.

[FKouhai]

yep I like that :)

[MattSturgeon]

Same optional/subjective suggestion:

Suggested change

	plugins.oil.settings={
	${plugins.oil.settings} = {

Also, nit: spaces around the = please 🙂

[MattSturgeon]

Q: is this the only supported valid string, or are there other values that'd work?

E.g. would it be more correct to check lib.strings.hasInfix "yes:2" signcolumn?

I'm unfamiliar with oil, but the value looks suspiciously like something that can be configured with more complex syntax and still work for oil-git-status?

[FKouhai]

so 2 would be min number of sign columns

[MattSturgeon]

Is it valid to e.g. have something like "yes:2,3" or "yes:2,10" to have a min & max?

If so, maybe we want lib.strings.hasPrefix "yes:2,"?

[FKouhai]

I don't think that's valid . the generated lua is this

 require("oil").setup({ win_options = { signcolumn = "yes:2" } })

it has to be a number bigger than 2 I think I could skip that , to avoid confusion

[FKouhai]

maybe we could use lib.strings,hasPrefix "yes:"

[FKouhai]

I just tried to run the tests without the , and they pass as well so I'd saw we are good

[FKouhai]

[MattSturgeon]

once it gets generated to lua it's already stripped

Is there a custom option apply function doing that? 🤔

[FKouhai]

might be part of the translation layer, like since in this case it's only that value being set it might just get stripped to prevent a typo or something like that

[MattSturgeon]

I don't see any custom behaviour in the option declaration:

nixvim/plugins/by-name/oil/default.nix

Line 249 in 36d63a7

signcolumn = helpers.defaultNullOpts.mkStr "no" "";

might be part of the translation layer,

If you mean the lua translation layer, that's not relevant to your warning. You are reading the option value (i.e. a nix value) not the rendered lua.

[MattSturgeon]

LGTM, one whitespace nit and a rebase needed, then I think its good to merge.

Once the whitespace is fixed, can you fetch our repo and rebase onto main?

git fetch --all
git rebase upstream/main
git push --force-with-lease

Thanks for your hard work on this!

[FKouhai]

LGTM, one whitespace nit and a rebase needed, then I think its good to merge.

Once the whitespace is fixed, can you fetch our repo and rebase onto main?

git fetch --all
git rebase upstream/main
git push --force-with-lease

Thanks for your hard work on this!

nah man thank you for bearing with me, sometimes I can be a bit intense, and thanks for the reviews they were extremely helpful and I got to learn a lot from this :)

[MattSturgeon]

@Mergifyio queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at aa1ae69

[mergify]

This pull request, with head sha aa1ae69b573e64ce145672663471795daed2ec9e, has been successfully merged with fast-forward by Mergify.

This pull request will be automatically closed by GitHub.

As soon as GitHub detects that the sha aa1ae69b573e64ce145672663471795daed2ec9e is part of the main branch, it will mark this pull request as merged.

It is possible for this pull request to remain open if this detection does not happen, this usually happens when a force-push is done on this branch feat/add_oil_git_status, this means GitHub will fail to detect the merge.