-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
ui/colorizer: rebase on maintained fork #203
Conversation
Looks good. Though, I've noticed that nvim-colorizer has been abandoned for a while (4 years at this point) with a maintained fork available with more intuitive configuration options. Could I ask you to rebase on the maintained fork? It'd save us the trouble of revisiting this plugin later just to change plugins. |
Nice catch, didn't notice that myself. I was too focused on just adding to what was already there, without making any drastic changes. I can do a rebase on the fork tomorrow morning. |
@horriblename The maintained fork of the plugin configures the filetypes with a table constructor for lists: filetypes = {
'css',
'javascript',
html = { mode = 'foreground'; }
} Regarding this setup, I have noticed that you have an option to collapse a key value pair to just value in |
I actually didn't add that, maybe it was @NotAShelf? in any case, it might be a bit better to expose a function like this luaTableWithSequence ["javascript" "java"] {html = {...}} I'm not sure how to best expose it as an option though in this particular case, it should be fine just writing everything as {
javascript = {} ;
css = {};
html = {mode =... ;} ;
} edit: ^^ i havent tested this no matter what we do here it's gonna suck for the user, I recommend just putting the empty tables like above instead of using custom syntax |
b2ab317
to
8b520fd
Compare
8b520fd
to
f94e6e1
Compare
dfa7c42
to
b0b3c34
Compare
This newest solution is comparable to what @horriblename suggested. If there's anything you would like changed, let me know. |
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.
LGTM
@NotAShelf Most of the options for the colorizer are wrapped inside the Other than that, it looks good to me. 👍 |
I believe this is a fair point. @Donnerinoern could you move filetypes outside options before I merge this? |
b0b3c34
to
5bbe75c
Compare
5bbe75c
to
d0f8e44
Compare
Done. |
Will merge once the CI passes. |
Thanks! |
ui/colorizer: rebase on maintained fork
Description
Added a new option for colorizer which, when enabled, will attach colorizer to any buffer with a filetype set in autoAttach.filetypes. I also fixed the indentation on colorizer's DEFAULT_OPTIONS.Rebased colorizer.nvim on a maintained fork and updated the options accordingly.
Type of change
Checklist
nix fmt
).Screenshots & Logs