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

plugins/conform: init + test #667

Merged
merged 3 commits into from
Nov 3, 2023
Merged

plugins/conform: init + test #667

merged 3 commits into from
Nov 3, 2023

Conversation

txtyash
Copy link

@txtyash txtyash commented Oct 24, 2023

Adds support for the conform-nvim plugin.
I could not test this due to conform-nvim not being in nixpkgs-unstable yet.
I'll amend further changes.

Comment on lines 58 to 62
logLevel =
helpers.defaultNullOpts.mkNullable
types.str
"vim.log.levels.ERROR"
" See :h log_levels ";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do the same thing as here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct now:

      logLevel =
        helpers.ifNonNull' level
        (helpers.mkRaw "vim.log.levels.${strings.toUpper level}");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems correct to me

Copy link
Author

@txtyash txtyash Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems correct to me

@GaetanLepage pointed out on matrix that this isn't how it should be done. This is what they said:

Hmmm
Not really
here is where you declare your option, so no "vim.log.levels." in front
Have I sent you an example ?

I suppose he meant that the options should not be hardcoded like this and it should only describe the datatype.

@GaetanLepage
Copy link
Member

@evccyr You may now rebase on the main branch. The flake.lock has been updated and conform.nvim is now available.

@traxys
Copy link
Member

traxys commented Oct 28, 2023

You include the file as conform-nvim.nix in the plugins/default.nix file, while you renamed the file to conform.nix

@txtyash
Copy link
Author

txtyash commented Oct 28, 2023

You include the file as conform-nvim.nix in the plugins/default.nix file, while you renamed the file to conform.nix

I'm just struggling with the logLevel for now. How should I do that?

Copy link
Member

@traxys traxys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you avoid using types.attrs and use types.attrsOf types.anything? Could could look it up online, attrs are not recursively merged, whereas types.attrsOf can be.

plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
tests/test-sources/plugins/lsp/conform.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
plugins/lsp/conform-nvim.nix Outdated Show resolved Hide resolved
@traxys traxys merged commit 7629115 into nix-community:main Nov 3, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants