-
-
Notifications
You must be signed in to change notification settings - Fork 294
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/fidget: migrate to mkNeovimPlugin #2647
base: main
Are you sure you want to change the base?
plugins/fidget: migrate to mkNeovimPlugin #2647
Conversation
HeitorAugustoLN
commented
Dec 12, 2024
•
edited
Loading
edited
- Migrated fidget.nvim to mkNeovimPlugin
- Added renames from old options to new ones (with the exception of the submodules, because I didn't find a way to)
- Added new options, etc.
- Updated tests to new format too
ce3e947
to
a37c6e3
Compare
2223c95
to
e433354
Compare
5992d4b
to
063fb6b
Compare
2ae72d4
to
1f1f01f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling one of the more bloated plugins, should be good overall.
c0a0789
to
73c7489
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like some minor tidying up and it should be good from my perspective. Thanks for working on this!
plugins/by-name/fidget/default.nix
Outdated
"pollRate" | ||
"suppressOnInsert" | ||
"ignoreDoneAlready" | ||
"ignoreEmptyMessage" | ||
"notificationGroup" | ||
"clearOnDetach" | ||
"ignore" | ||
]; | ||
progressDisplayOptions = [ | ||
"renderLimit" | ||
"doneTtl" | ||
"doneIcon" | ||
"doneStyle" | ||
"progressTtl" | ||
"progressIcon" | ||
"progressStyle" | ||
"groupStyle" | ||
"iconStyle" | ||
"priority" | ||
"skipHistory" | ||
"formatMessage" | ||
"formatAnnote" | ||
"formatGroupName" | ||
"overrides" | ||
]; | ||
notificationOptions = [ | ||
"pollRate" | ||
"filter" | ||
"historySize" | ||
"overrideVimNotify" | ||
"configs" | ||
"redirect" | ||
]; | ||
notificationViewOptions = [ | ||
"stackUpwards" | ||
"iconSeparator" | ||
"groupSeparator" | ||
"groupSeparatorHl" | ||
]; | ||
notificationWindowOptions = [ | ||
"normalHl" | ||
"winblend" | ||
"border" | ||
"borderHl" | ||
"zindex" | ||
"maxWidth" | ||
"maxHeight" | ||
"xPadding" | ||
"yPadding" | ||
"align" | ||
"relative" | ||
]; | ||
in | ||
[ | ||
[ | ||
"progress" | ||
"lsp" | ||
"progressRingbufSize" | ||
] | ||
[ | ||
"integration" | ||
"nvim-tree" | ||
"enable" | ||
] | ||
[ | ||
"logger" | ||
"level" | ||
] | ||
[ | ||
"logger" | ||
"floatPrecision" | ||
] | ||
[ | ||
"logger" | ||
"path" | ||
] | ||
] | ||
++ (map (oldOption: [ | ||
"progress" | ||
oldOption | ||
]) progressOptions) | ||
++ (map (oldOption: [ | ||
"progress" | ||
"display" | ||
oldOption | ||
]) progressDisplayOptions) | ||
++ (map (oldOption: [ | ||
"notification" | ||
oldOption | ||
]) notificationOptions) | ||
++ (map (oldOption: [ | ||
"notification" | ||
"view" | ||
oldOption | ||
]) notificationViewOptions) | ||
++ (map (oldOption: [ | ||
"notification" | ||
"window" | ||
oldOption | ||
]) notificationWindowOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice for this to also be in deprecations.nix
.
Perhaps we could restructure that file, so that instead of being a module it is an attrset with optionsRenamedToSettings
and imports
that can then be used in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented it, don't know if I did the right way though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is good now
plugins/by-name/fidget/default.nix
Outdated
++ (map (oldOption: [ | ||
"progress" | ||
oldOption | ||
]) progressOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: these outer parens are not needed in concatenation. Also, the new nixfmt rules should allow the list to be on one line, because it is being applied as a function-arg:
++ (map (oldOption: [ | |
"progress" | |
oldOption | |
]) progressOptions) | |
++ map (oldOption: [ "progress" oldOption ]) progressOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the current package version is not up-to-date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nixfmt is definitely up to date. Could just be the line ends up being too long, so the formatter insists on having a multiline list.
mkRemovedOptionModule | ||
[ | ||
"plugins" | ||
"fidget" | ||
oldOption | ||
] | ||
'' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure, but I think the new nixfmt rules allow us to use:
mkRemovedOptionModule | |
[ | |
"plugins" | |
"fidget" | |
oldOption | |
] | |
'' | |
mkRemovedOptionModule [ "plugins" "fidget" oldOption ] '' |
d39848c
to
e6db071
Compare
e6db071
to
8e5c1d4
Compare