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

feat(shfmt): add automatic indentation detection #481

Merged
merged 4 commits into from
Jul 1, 2024

Conversation

mehalter
Copy link
Contributor

@mehalter mehalter commented Jun 28, 2024

This adds the automatic setting of -i so that shfmt follows the current indentation settings of neovim. Whether it is manually set by the user or set through external factors such as .editorconfig

I also checked to see if this is done elsewhere for other formatters and found a couple others which I was able to improve the "correctness" of their indentation size calculations

EDIT: I was thinking about this and it seems like having the effective shiftwidth value is quite nice. And because this seems to be "contextual information" it seemed natural to put it in the conform.Context type since it is virtually free to calculate performance wise. Let me know if you disagree. Another approach would be to make some sort of utility module that is imported by formatters on an as needed basis that calculates this. This is basically the same as vim.fn.shiftwidth (:h shiftwidth()) with the only difference being that it calculates it specifically for the buffer in the context rather than the "current buffer". This allowed for a very nice refactor across the formatters that utilize this field and is available for future formatters. Also nice to make sure we have a common method for calculating indentation size.

Also shiftwidth and tabstop are always integers according to the neovim docs so no need to have default fallbacks or check for nil

@github-actions github-actions bot requested a review from stevearc June 28, 2024 13:33
@mehalter
Copy link
Contributor Author

mehalter commented Jun 28, 2024

If this is a common task, it might be useful to provide this maybe as a utility so formatters can easily calculate it on demand or we could even put this as part of the formatter context that is passed to the args and prepend_args functions

Mainly just the part that does the logic for calculating the correct indentation size. Something like expandtab is not necessary. But how the tab key is converted to some number of spaces is not based just on tabstop

@mehalter mehalter force-pushed the shfmt_indent_size branch 3 times, most recently from 6d457e2 to 68237db Compare June 28, 2024 14:23
this also refactors formatters that automatically set indentation level
to use the new shiftwidth context
@mehalter mehalter force-pushed the shfmt_indent_size branch from 68237db to a9bfedf Compare June 28, 2024 14:30
@stevearc
Copy link
Owner

stevearc commented Jul 1, 2024

Nice QOL improvement for people writing formatters. Thanks for the well reasoned PR!

@stevearc stevearc merged commit cd75be8 into stevearc:master Jul 1, 2024
9 checks passed
@mehalter mehalter deleted the shfmt_indent_size branch July 1, 2024 18:03
@tigion tigion mentioned this pull request Jul 17, 2024
1 task
octvs added a commit to octvs/deepl that referenced this pull request Aug 25, 2024
- Make local indentation on `sh` (and `bash`) filetypes equal to two
  spaces, following google style guide[1].
- Make our formatter setup work with the upstream change [2], by
  removing manually set `shiftwidth` through argument `-i` to shfmt.
  Instead set the indentation via an auto command on these filetypes.
- Format local scripts to have correct indentation.

[1]: https://google.github.io/styleguide/shellguide.html
[2]: stevearc/conform.nvim#481
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