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

neovim: use mini.base16 for colorscheme #536

Merged
merged 2 commits into from
Oct 19, 2024
Merged

Conversation

soulsoiledit
Copy link
Contributor

Switches both the neovim and nixvim modules to use mini.base16.

Closes #535

Copy link
Collaborator

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

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

From 6fe4ff7a686ae66a23c8040bb4384eccf801aa85 Mon Sep 17 00:00:00 2001
From: soulsoiledit <[email protected]>
Date: Wed, 28 Aug 2024 18:18:16 -0500
Subject: [PATCH 1/2] nixvim: use mini.base16 for colorscheme

mini.base16 provides better support for various neovim plugins

The commit message could be improved to:

nixvim: set colorscheme using mini.base16

Set the colorscheme using the mini.base16 plugin for better plugin integration.

Link: https://github.com/danth/stylix/pull/536
---
 modules/nixvim/nixvim.nix | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/modules/nixvim/nixvim.nix b/modules/nixvim/nixvim.nix
index dbc1da66..f6f20bf5 100644
--- a/modules/nixvim/nixvim.nix
+++ b/modules/nixvim/nixvim.nix
@@ -28,14 +28,14 @@
   config = lib.mkIf (config.stylix.enable && config.stylix.targets.nixvim.enable && (config.programs ? nixvim)) (
     lib.optionalAttrs (builtins.hasAttr "nixvim" options.programs) {
       programs.nixvim = {
-        colorschemes.base16 = {
-          colorscheme = {
+        plugins.mini = {
+          enable = true;
+
+          modules.base16.palette = {
             inherit (config.lib.stylix.colors.withHashtag)
               base00 base01 base02 base03 base04 base05 base06 base07
               base08 base09 base0A base0B base0C base0D base0E base0F;
           };
-
-          enable = true;
         };

         highlight = let

Unless Nixvim refuses this patch, I would prefer this to be upstreamed to Nixvim, and then we simply update the Nixvim input.

This patch causes the following error without any additional inputs.stylix.inputs.<INPUTS>.follows declarations:

Error detected while processing /home/<USER>/.config/nvim/init.lua:
E5113: Error while calling lua chunk: /home/<USER>/.config/nvim/init.lua:81: module 'mini.base16' not found:
        no field package.preload['mini.base16']
        no file '/nix/store/kf5k5qa53g9rrsch256vqgnr7al080al-luajit-2.1.1693350652-env/share/lua/5.1/mini/base16.lua'
        no file '/nix/store/kf5k5qa53g9rrsch256vqgnr7al080al-luajit-2.1.1693350652-env/share/lua/5.1/mini/base16/init.lua'
        no file '/nix/store/kf5k5qa53g9rrsch256vqgnr7al080al-luajit-2.1.1693350652-env/lib/lua/5.1/mini/base16.so'
        no file '/nix/store/kf5k5qa53g9rrsch256vqgnr7al080al-luajit-2.1.1693350652-env/lib/lua/5.1/mini.so'
stack traceback:
        [C]: in function 'require'
        /home/<USER>/.config/nvim/init.lua:81: in main chunk

Maybe some Stylix inputs need to be updated.

From 1e8a1c103bd80c4bcfdbf649db84f71d47df6a53 Mon Sep 17 00:00:00 2001
From: soulsoiledit <[email protected]>
Date: Wed, 28 Aug 2024 18:20:36 -0500
Subject: [PATCH 2/2] neovim: use mini.base16 for colorscheme

mini.base16 provides better support for various neovim plugins

The commit message could be improved to:

neovim: replace base16-nvim with mini.base16

Replace the base16-nvim plugin with the mini.base16 plugin for better plugin
integration.

Closes: https://github.com/danth/stylix/issues/535
Link: https://github.com/danth/stylix/pull/536
---
 modules/neovim/hm.nix | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/modules/neovim/hm.nix b/modules/neovim/hm.nix
index 587c196e..e565ad1d 100644
--- a/modules/neovim/hm.nix
+++ b/modules/neovim/hm.nix
@@ -18,15 +18,17 @@
       in
       {
         plugins = lib.singleton {
-          plugin = pkgs.vimPlugins.base16-nvim;
+          plugin = pkgs.vimPlugins.mini-nvim;
           type = "lua";
           config = lib.mkMerge [
             (with config.lib.stylix.colors.withHashtag; ''
-              require('base16-colorscheme').setup({
-                base00 = '${base00}', base01 = '${base01}', base02 = '${base02}', base03 = '${base03}',
-                base04 = '${base04}', base05 = '${base05}', base06 = '${base06}', base07 = '${base07}',
-                base08 = '${base08}', base09 = '${base09}', base0A = '${base0A}', base0B = '${base0B}',
-                base0C = '${base0C}', base0D = '${base0D}', base0E = '${base0E}', base0F = '${base0F}'
+              require('mini.base16').setup({
+                palette = {
+                  base00 = '${base00}', base01 = '${base01}', base02 = '${base02}', base03 = '${base03}',
+                  base04 = '${base04}', base05 = '${base05}', base06 = '${base06}', base07 = '${base07}',
+                  base08 = '${base08}', base09 = '${base09}', base0A = '${base0A}', base0B = '${base0B}',
+                  base0C = '${base0C}', base0D = '${base0D}', base0E = '${base0E}', base0F = '${base0F}'
+                }
               })
             '')
             (lib.mkIf cfg.transparentBackground.main ''

I have not tested this patch yet.

@donovanglover
Copy link
Contributor

This is how base16.nvim looks for me:

image

And this is how mini.nvim looks:

image

Making this change optional would be nice.

@trueNAHO
Copy link
Collaborator

Making this change optional would be nice.

We could add options to enable specific color scheme setters.

@soulsoiledit, can you elaborate on how the algorithms of base16.nvim and mini.nvim plugins technically differ? For example, the mini.nvim plugin already explains its algorithm: https://github.com/echasnovski/mini.base16/blob/95db7f0ea89fbc60b321dbe23d39c1502a07a20e/lua/mini/base16.lua#L223-L257. Based on this comparison we can decide which one should be the default.

@soulsoiledit
Copy link
Contributor Author

In this context, the algorithms don't differ at all. The algorithm described in mini-base16 only applies when you want to provide a subset of colors, ie only background, foreground, and accent. In that case, it generates a suitable palette based on those colors.

Since we are providing the full palette, mini.base16 calls this function (https://github.com/echasnovski/mini.base16/blob/95db7f0ea89fbc60b321dbe23d39c1502a07a20e/lua/mini/base16.lua#L446) instead, which does the same thing as base16-nvim, grabbing colors directly from the palette given.

I'm fine with keeping base16-nvim as the default.

@trueNAHO
Copy link
Collaborator

trueNAHO commented Aug 29, 2024

Since we are providing the full palette, mini.base16 calls this function (https://github.com/echasnovski/mini.base16/blob/95db7f0ea89fbc60b321dbe23d39c1502a07a20e/lua/mini/base16.lua#L446) instead, which does the same thing as base16-nvim, grabbing colors directly from the palette given.

In that case, why do the colors look different:

This is how base16.nvim looks for me:

image

And this is how mini.nvim looks:

image

Is it because some plugins are explicitly themed by mini.nvim?

I'm fine with keeping base16-nvim as the default.

If the premise of mini.nvim is to be base16.nvim with additional plugin support, then it might be better to make mini.nvim the default.

@soulsoiledit
Copy link
Contributor Author

mini assigns that highlight group, builtin variables such as (this, self, etc), as part of "special symbols" within a language, while base16-nvim assigns them the same as other variables.

@trueNAHO
Copy link
Collaborator

In that case, why do the colors look different:

[...]

Is it because some plugins are explicitly themed by mini.nvim?

mini assigns that highlight group, builtin variables such as (this, self, etc), as part of "special symbols" within a language, while base16-nvim assigns them the same as other variables.

The visible change is probably caused by mini.nvim's intervention with Treesitter. Just search for tree in https://github.com/echasnovski/mini.base16/blob/95db7f0ea89fbc60b321dbe23d39c1502a07a20e/lua/mini/base16.lua.

soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Aug 30, 2024
Replace the base16-nvim plugin with the mini.base16 plugin for better
plugin integration

Link: danth#536
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Aug 30, 2024
Replace the base16-nvim plugin with the mini.base16 plugin for better
plugin integration

Link: danth#536
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Aug 30, 2024
Add an option to select whether to use 'mini.base16' or 'base16-nvim'.

Link: danth#536
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Aug 30, 2024
Add an option to select whether to use 'mini.base16' or 'base16-nvim'.

Link: danth#536
@soulsoiledit
Copy link
Contributor Author

This patch causes the following error without any additional inputs.stylix.inputs..follows declarations:

I'm not sure what's going on here. I checked the pinned version of nixpkgs in stylix, and the mini.nvim version there does seem to have mini.base16 included.

Making this change optional would be nice.

I have added this option under stylix.targets.neovim.plugin and stylix.targets.nixvim.plugin.

trueNAHO added a commit to soulsoiledit/stylix that referenced this pull request Aug 30, 2024
Add the stylix.targets.neovim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
trueNAHO added a commit to soulsoiledit/stylix that referenced this pull request Aug 30, 2024
Add the stylix.targets.nixvim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
@trueNAHO
Copy link
Collaborator

This patch causes the following error without any additional inputs.stylix.inputs..follows declarations:

I'm not sure what's going on here. I checked the pinned version of nixpkgs in stylix, and the mini.nvim version there does seem to have mini.base16 included.

There is probably something wrong with my setup. I will try to test this tomorrow.

I have added this option under stylix.targets.neovim.plugin and stylix.targets.nixvim.plugin.

I performed the following changes to this PR:

  • Properly merge commit 8b6d334 ("neovim: add plugin selection option") into the commits 75394d9 ("nixvim: set colorscheme with mini.base16") and d587901 ("neovim: set colorscheme with mini.base16")
  • Rename stylix.targets.neovim.plugin to stylix.targets.neovim.colorscheme
  • Improve stylix.targets.neovim.colorscheme description

trueNAHO added a commit to soulsoiledit/stylix that referenced this pull request Aug 31, 2024
Add the stylix.targets.nixvim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
@trueNAHO
Copy link
Collaborator

trueNAHO commented Aug 31, 2024

I will try to test this tomorrow.

I added my Tested-by tag to commit 7d5d9a4 ("nixvim: add stylix.targets.nixvim.colorscheme option").

@danth
Copy link
Owner

danth commented Aug 31, 2024

In that case, why do the colors look different

Ideally the color usage should be as close to the upstream style guide as possible. Our own style guide currently just refers to that link when it comes to text editors. Unfortunately, the guide isn't very specific in some cases, such as the example with this and self, hence the differences as each plugin author has used their own preference.

From the screenshots above, I personally prefer this new plugin since it differentiates between variables and boolean values in Nix, although that's just one of I assume many differences.

image
image

According to the upstream guide, nether of these are totally correct - variables should be red and booleans should be yellow (unless @donovanglover's chosen scheme has non-standard colors).

@trueNAHO
Copy link
Collaborator

trueNAHO commented Sep 1, 2024

Ideally the color usage should be as close to the upstream style guide as possible. Our own style guide currently just refers to that link when it comes to text editors. Unfortunately, the guide isn't very specific in some cases, such as the example with this and self, hence the differences as each plugin author has used their own preference.

[...]

According to the upstream guide, nether of these are totally correct - variables should be red and booleans should be yellow (unless @donovanglover's chosen scheme has non-standard colors).

Until the style guide has been expanded 1, it won't hurt to provide a new colorscheme plugin. Once the VIM highlight groups are implemented in Stylix 1, we can replace both colorscheme plugins with the VIM highlight groups.

@donovanglover
Copy link
Contributor

I think the colors are different due to nvim-lspconfig and treesitter. I mainly like base16-nvim for its usage of italics and other style differences (like export being blue in JS/TS) but it does seem different than the spec.

image

Theme is selenized-black if anyone wants to try it.

soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 4, 2024
Add the stylix.targets.neovim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 4, 2024
Add the stylix.targets.nixvim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 19, 2024
Add the stylix.targets.neovim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 19, 2024
Add the stylix.targets.nixvim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
Copy link
Owner

@danth danth left a comment

Choose a reason for hiding this comment

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

Other than my comment below, this looks good to go :))

modules/neovim/hm.nix Outdated Show resolved Hide resolved
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 29, 2024
Add the stylix.targets.neovim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 29, 2024
Add the stylix.targets.nixvim.colorscheme option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 29, 2024
Add the stylix.targets.neovim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 29, 2024
Add the stylix.targets.nixvim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 29, 2024
Add the stylix.targets.neovim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 29, 2024
Add the stylix.targets.nixvim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 30, 2024
Add the stylix.targets.neovim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
soulsoiledit added a commit to soulsoiledit/stylix that referenced this pull request Sep 30, 2024
Add the stylix.targets.nixvim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
Copy link
Collaborator

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

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

I reviewed both commits and re-tested the Nixvim patch.

@trueNAHO
Copy link
Collaborator

trueNAHO commented Oct 7, 2024

@danth, can you do a merge commit for this PR? AFAIK, I only have permission to do squash commits via the GitHub UI.

soulsoiledit and others added 2 commits October 12, 2024 12:54
Add the stylix.targets.neovim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Add the stylix.targets.nixvim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
@danth danth merged commit 6616348 into danth:master Oct 19, 2024
10 checks passed
@danth
Copy link
Owner

danth commented Oct 19, 2024

@trueNAHO done :)

@soulsoiledit soulsoiledit deleted the mini.base16 branch October 20, 2024 00:32
Mikilio pushed a commit to Mikilio/stylix that referenced this pull request Nov 4, 2024
Add the stylix.targets.neovim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Mikilio pushed a commit to Mikilio/stylix that referenced this pull request Nov 4, 2024
Add the stylix.targets.nixvim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
Mikilio pushed a commit to Mikilio/stylix that referenced this pull request Nov 4, 2024
Add the stylix.targets.neovim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Mikilio pushed a commit to Mikilio/stylix that referenced this pull request Nov 4, 2024
Add the stylix.targets.nixvim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
mateusauler pushed a commit to mateusauler/stylix that referenced this pull request Nov 17, 2024
Add the stylix.targets.neovim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
mateusauler pushed a commit to mateusauler/stylix that referenced this pull request Nov 17, 2024
Add the stylix.targets.nixvim.plugin option to select between the
previous base16-nvim and the new default mini.base16 [1] plugin,
offering better plugin integration.

[1]: https://github.com/echasnovski/mini.base16

Link: danth#536

Co-authored-by: NAHO <[email protected]>
Tested-by: NAHO <[email protected]>
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.

neovim: support mini.base16
4 participants