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

Add .editorconfig support for shfmt #1171

Merged
merged 8 commits into from
Jun 10, 2024

Conversation

chris-reeves
Copy link
Contributor

@chris-reeves chris-reeves commented Jun 7, 2024

This PR adds support for reading shfmt config via .editorconfig.

If any shfmt-specific configuration properties are found in .editorconfig then the config in .editorconfig will be used and the language server config will be ignored. This follows shfmt's approach of using either .editorconfig or command line flags, but not both. Note that only shfmt-specific configuration properties are read from .editorconfig - indentation preferences are still provided by the editor, so to format using the indentation specified in .editorconfig make sure your editor is also configured to read .editorconfig.

It is possible to disable .editorconfig support and always use the language server config by setting the "Ignore Editorconfig" configuration variable.

See #1165 for some discussion of the approach to this.

Language dialect is something that can be set via `.editorconfig`, so
we'll need to respect that when running `shfmt`.
If we have any `shfmt`-specific options in `.editorconfig`, use the
config in `.editorconfig` and ignore the language server config (this is
similar to `shfmt`'s approach of using either `.editorconfig` or command
line flags, but not both).

Indentation always comes via the editor - if someone is using
`.editorconfig` then the expectation is that they will have configured
their editor's indentation in this way too.
The error message that we expect from one of our tests is different
depending upon which version of shfmt is being run. Fix the test to
accept both error messages.
Copy link

codecov bot commented Jun 8, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 81.09%. Comparing base (3a5ac07) to head (bbfa220).
Report is 1 commits behind head on main.

Current head bbfa220 differs from pull request most recent head 986548d

Please upload reports for the commit 986548d to get more accurate results.

Files Patch % Lines
server/src/shfmt/index.ts 80.00% 0 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1171      +/-   ##
==========================================
- Coverage   81.27%   81.09%   -0.18%     
==========================================
  Files          29       29              
  Lines        1415     1439      +24     
  Branches      338      342       +4     
==========================================
+ Hits         1150     1167      +17     
  Misses        218      218              
- Partials       47       54       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chris-reeves
Copy link
Contributor Author

Fixed one of the tests to work with the older version of shfmt available on the GitHub runners.

@skovhus skovhus enabled auto-merge June 10, 2024 06:53
@skovhus skovhus merged commit 23fdaa9 into bash-lsp:main Jun 10, 2024
4 checks passed
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.

2 participants