search

google / vim-codefmt

Vim plugin for syntax-aware code formatting
Apache License 2.0
1.11k stars 114 forks source link

Add --lint_mode flag for buildifier #192

Closed TamaMcGlinn closed 2 years ago

TamaMcGlinn commented 2 years ago

In your vimrc you can do e.g.

call glaive#Install()
Glaive codefmt buildifier_lint_mode='fix'

to get buildifier to autofix issues. The default behaviour is unchanged.

dbarnett commented 2 years ago

Wouldn't it be better to by default not pass any mode override at all? This would be another layer of defaulting that could fight with other local config settings, etc.

TamaMcGlinn commented 2 years ago

I'm passing the default value though, so that will do exactly the same. Anyway, I like being explicit about the lint_mode. If this could fight with other local config settings, that means you're saying the lint_mode can already be specified in some other way, and the whole PR would be unnecessary. As far as I know, that's not the case, but if so, please elaborate on how to do that.

dbarnett commented 2 years ago

I don't know whether buildifier would have any other override mechanism in practice or not, just that I prefer not to copy-paste defaults. IOW, you're saying it will do the same because you're passing the same default value… what if they change the default later, like add a new middle option for minimal fixes and make that the default? Would we want this default to take precedence over theirs?

TamaMcGlinn commented 2 years ago

okay fair enough. so it should be changed to have the empty string as the default, and an if-statement added to not add the parameter in that case.

TamaMcGlinn commented 2 years ago

Okay, fixed that. Also tested and specified in the help message what the behaviour is for 'warn', since it is possible someone would actually choose that behaviour.

dbarnett commented 2 years ago

Thanks for working with me on the defaults! Looks good now besides one .. thing that might be broken I commented on.

dbarnett commented 2 years ago

Merged, thanks!