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/lualine: migrate to mkNeovimPlugin #2080

Merged
merged 1 commit into from
Sep 7, 2024

Conversation

khaneliman
Copy link
Contributor

@khaneliman khaneliman commented Aug 25, 2024

Resolves #2191

Migrating to the current standards. Required using a transitionType for the component options that will help keep existing configurations working, but recommend switching to the new way of being explicit in your configuration to match upstream plugin declaration.

@khaneliman

This comment was marked as outdated.

@MattSturgeon

This comment was marked as resolved.

@khaneliman khaneliman force-pushed the lualine branch 2 times, most recently from e0df900 to ee04c75 Compare September 6, 2024 01:02
@khaneliman

This comment was marked as resolved.

plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
@MattSturgeon

This comment was marked as resolved.

@khaneliman

This comment was marked as resolved.

@khaneliman khaneliman force-pushed the lualine branch 3 times, most recently from aaed678 to f7dd21f Compare September 7, 2024 15:18
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
@khaneliman khaneliman force-pushed the lualine branch 2 times, most recently from 60a1e8a to eba101e Compare September 7, 2024 15:33
@khaneliman khaneliman force-pushed the lualine branch 5 times, most recently from 18457e0 to 15b259e Compare September 7, 2024 18:22
@khaneliman khaneliman force-pushed the lualine branch 2 times, most recently from 12634d2 to 06ad4c0 Compare September 7, 2024 18:39
@khaneliman khaneliman force-pushed the lualine branch 3 times, most recently from 0657e16 to d25d928 Compare September 7, 2024 18:46
Copy link
Member

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

Everything looks good, IDK if you want a second opinion, but you've been very thorough in checking upstream options so I trust your changes 🚀

plugins/statuslines/lualine.nix Outdated Show resolved Hide resolved
@khaneliman
Copy link
Contributor Author

Everything looks good, IDK if you want a second opinion, but you've been very thorough in checking upstream options so I trust your changes 🚀

Thanks, I'm fairly confident in the changes since it works with my config and I'd consider it robust. Was just double checking the diff a bit and everything looks pretty good.

@khaneliman
Copy link
Contributor Author

@Mergifyio queue

Copy link
Contributor

mergify bot commented Sep 7, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at d12045e

@mergify mergify bot merged commit d12045e into nix-community:main Sep 7, 2024
4 checks passed
@khaneliman khaneliman deleted the lualine branch September 7, 2024 19:25
@SuperSandro2000
Copy link
Member

I don't think the mkRenamedOptions is working correctly here

       error: A definition for option `programs.nixvim.plugins.lualine.settings.sections.lualine_b' is not of type `null or (list of (string or ((attribute set of anything) or raw lua code))) or raw lua code'. Definition values:
       - In `/nix/store/y93lbhv6j469navzizc944gvgc5hcn4v-source/plugins/by-name/lualine':

@MattSturgeon
Copy link
Member

I don't think the mkRenamedOptions is working correctly here

       error: A definition for option `programs.nixvim.plugins.lualine.settings.sections.lualine_b' is not of type `null or (list of (string or ((attribute set of anything) or raw lua code))) or raw lua code'. Definition values:
       - In `/nix/store/y93lbhv6j469navzizc944gvgc5hcn4v-source/plugins/by-name/lualine':

Thanks for reporting. What does the offending definition look like?

@MattSturgeon
Copy link
Member

This doesn't make much sense, because the new type should be a superset of the old type.

# new type:
null or (list of (string or ((attribute set of anything) or raw lua code))) or raw lua code

# old type:
null or (list of (string or (submodule)))

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Sep 9, 2024

Thanks for reporting. What does the offending definition look like?

That is the old config https://github.com/NuschtOS/nvim.nix/blob/4aae65ffea0b488803245fd27e6ef5812faa5fa7/modules/default.nix#L79-L92

My attempt at updating is at NuschtOS/nvim.nix@d11ad57 (#47)

@khaneliman
Copy link
Contributor Author

khaneliman commented Sep 10, 2024

Thanks for reporting. What does the offending definition look like?

That is the old config https://github.com/NuschtOS/nvim.nix/blob/4aae65ffea0b488803245fd27e6ef5812faa5fa7/modules/default.nix#L79-L92

My attempt at updating is at NuschtOS/nvim.nix@d11ad57 (#47)

I created test cases for both configs and they worked: https://github.com/nix-community/nixvim/compare/main...khaneliman:nixvim:sandro?expand=1

image

Running the old config:
image

Testing and running the new config worked without the warnings. Wondering if there's something else going on then...

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.

[BUG] Lualine options don't behave as like actual lualine configuration
3 participants