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

syntax: fix two bugs when formatting case clauses #952

Merged
merged 2 commits into from
Dec 4, 2022
Merged

Conversation

mvdan
Copy link
Owner

@mvdan mvdan commented Dec 3, 2022

(see commit messages - please do not squash)

Fixes #779.
Fixes #917.

cc @ainar-g @Julien-Elie @zx8 @DannyBen if you want to test this out

mvdan added 2 commits December 3, 2022 09:54
If we remove any newlines when printing, don't insert others later on in
the source.

While here, improve how we keep track of p.line throughout the printer.

Fixes #779.
We weren't doing that when a case clause item used quoted strings,
due to the `p.tok == _LitWord` condition that shouldn't be there.
Add tests, docs, and also prevent SingleLine from producing bad syntax
when printing the new test.

Fixes #917.
@mvdan mvdan requested a review from riacataquian December 3, 2022 10:28
@DannyBen
Copy link

DannyBen commented Dec 3, 2022

Want. But I am only using the binary. I will try to build me a small Docker with it for testing.

@DannyBen
Copy link

DannyBen commented Dec 3, 2022

Beautiful.

I can confirm this branch works as expected for me. Tested with the test.sh file mentioned in #950

➜ shfmt -d -i 2 -ci test.sh

--- test.sh.orig
+++ test.sh
@@ -3,8 +3,7 @@
 action=download

 case $action in
-  -*)
-    ;;
+  -*) ;;

   upload)
     # This upload section is being messed up with a weird backslash

DannyBen added a commit to DannyBen/bashly that referenced this pull request Dec 3, 2022
@ainar-g
Copy link

ainar-g commented Dec 3, 2022

I can confirm that the issue with case's is fixed. (I seem to have found another issue, but that one seems to be unrelated to this PR, and I'll file it separately.) Thanks!

@mvdan mvdan merged commit 289e21b into master Dec 4, 2022
@mvdan mvdan deleted the printer-patches branch December 5, 2022 16:42
DannyBen added a commit to DannyBen/bashly that referenced this pull request Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants