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

Investigate if shfmt is something for us #94

Open
kvz opened this issue Sep 5, 2017 · 5 comments
Open

Investigate if shfmt is something for us #94

kvz opened this issue Sep 5, 2017 · 5 comments

Comments

@kvz
Copy link
Owner

kvz commented Sep 5, 2017

https://github.com/mvdan/sh

@zbeekman
Copy link
Collaborator

zbeekman commented Sep 5, 2017

I use it, but there are some problems... mostly having to do with arithmetic expressions, and the inability to run shfmt if you need to dereference counter variables in arithmetic expressions... (Things like ${#} incorrectly either cause parsing errors or claim that bash doesn't support it, or says it's a style error, but there's no other way to access these variables without an intermediate temporary as far as I can tell... I've been meaning to file a bug report for some time but haven't gotten to it yet.)

It may also have some conflicting opinions about style compared to style.pl but I'm fine with either convention. It's nice that you can add a git hook so that it will reformat your code before checking it in.

@kvz
Copy link
Owner Author

kvz commented Sep 6, 2017

Seems you investigated already, nice 😄

Also seems it's not a clear 💯 yet, perhaps let's wait a bit for the project to mature before investing in it. Will leave this open so we won't forget then

@mvdan
Copy link

mvdan commented Sep 6, 2017

Hi! Shamelessly adding myself to the conversation.

It's entirely possible that parameter and arithmetic expansions have bugs - it's hard to get all the edge cases and details right. Have you tried recent versions such as 2.0? I fixed quite a bit of these earlier this year, so for example ${#} now works.

Of course it's getting better with time, but it will get better faster if you spot bugs and report them :)

Also note that a few things can be changed about its style - indentation, binary operator positioning and switch case double-indents as of now.

@zbeekman
Copy link
Collaborator

zbeekman commented Sep 6, 2017

@mvdan Thanks for chiming in! I'll have to test again with the latest version. If anything still seems broken I'll file a bug report.

Also, yes, I know I should not "suffer in silence" and know how frustrating this can be as an open source maintainer myself. Just hard to attend to all of life's priorities

@mvdan
Copy link

mvdan commented Sep 6, 2017

No worries :) You don't need to provide detailed bug reports if you don't have the time - a way to reproduce the issue is enough.

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

No branches or pull requests

3 participants