Allow shfmt fixer to use .editorconfig
#4244
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hbarcelos
changed the title
shfmt to use .editorconfigshfmt fixer to use .editorconfig
hsanson
approved these changes
Jul 2, 2022
cyyever
pushed a commit
to cyyever/ale
that referenced
this pull request
Jul 11, 2022
* fix: added support for local solhint executable * feat: added support for matching parse errors * test: added test for solhint command callback and handler * chore: removed command callback test * refactor: made solhint handler structure closer to eslint * refactor(shfmt-fixer): remove derivation of default CLI arguments
cyyever
pushed a commit
to cyyever/ale
that referenced
this pull request
Jul 11, 2022
* fix: added support for local solhint executable * feat: added support for matching parse errors * test: added test for solhint command callback and handler * chore: removed command callback test * refactor: made solhint handler structure closer to eslint * refactor(shfmt-fixer): remove derivation of default CLI arguments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Fix #4233]
Motivation
As discussed in #4233,
shfmthas some peculiarities that prevent it from working seamlessly standalone and from within ALE.Currently, the
shfmtfixer tries to derive the CLI arguments to pass to the executable fromexpandtab,shiftwidthandtabstop.While it seems like a good default at first sight,
shfmtdocs state:This means that the current fixer is currently preventing
shfmtfrom being customized on a more granular level with an.editorconfig. The worst part is that it's currently impossible to have project-level configuration forshfmtwithout resorting to things like local.vimrcs.I have reached out to the author of
shfmt(see issue #889) and came to the conclusion that the best approach is to use the-filenameoption to pass the original file path.I would also like to highlight this specific comment made by the author:
Given that this an implementation detail, I think the best approach is to treat it as a black-box. For that reason, I didn't try to add test cases covering the behavior of
shfmtwhen a.editorconfigis present, just stuck to its default behavior by default.Users are still free to configure their own CLI arguments through
g:ale_sh_shfmt_option, which will still disable.editorconfigsupport, but that is an opt-in behavior (maybe this should be documented somewhere).Commit List