Skip to content

Commit

Permalink
syntax: use fewer newlines in heredocs with minify
Browse files Browse the repository at this point in the history
Given the shell

  a=$(
    cat <<EOF
    hello
  EOF
  )

We would format it into

  a=$(cat \
  <<EOF)
    hello
  EOF

First, this makes bash unhappy, as it wants to find the closing EOF
before we close the command substitution. Teach rightParen to use a
newline if there are any pending heredocs, even if we're minifying.

Second, the escaped newline before the redirect is unnecessary.

Now, we format the shell into

  a=$(cat <<EOF
    hello
  EOF
  )

Thanks to Andrey Kaipov for the report and test cases.

Fixes #923.
  • Loading branch information
andreykaipov authored and mvdan committed Apr 15, 2023
1 parent a8d7768 commit ca2088c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
21 changes: 12 additions & 9 deletions syntax/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ func (p *Printer) spacePad(pos Pos) {
// wantsNewline reports whether we want to print at least one newline before
// printing a node at a given position. A zero position can be given to simply
// tell if we want a newline following what's just been printed.
func (p *Printer) wantsNewline(pos Pos) bool {
func (p *Printer) wantsNewline(pos Pos, escapingNewline bool) bool {
if p.mustNewline {
// We must have a newline here.
return true
Expand All @@ -319,7 +319,10 @@ func (p *Printer) wantsNewline(pos Pos) bool {
// as that might move them further down to the wrong place.
return false
}
// THe newline is optional, and we want it via either wantNewline or via
if escapingNewline && p.minify {
return false
}
// The newline is optional, and we want it via either wantNewline or via
// the position's line.
return p.wantNewline || pos.Line() > p.line
}
Expand Down Expand Up @@ -351,7 +354,7 @@ func (p *Printer) spacedToken(s string, pos Pos) {
}

func (p *Printer) semiOrNewl(s string, pos Pos) {
if p.wantsNewline(Pos{}) {
if p.wantsNewline(Pos{}, false) {
p.newline(pos)
p.indent()
} else {
Expand Down Expand Up @@ -509,7 +512,7 @@ func (p *Printer) newlines(pos Pos) {
p.firstLine = false
return // no empty lines at the top
}
if !p.wantsNewline(pos) {
if !p.wantsNewline(pos, false) {
return
}
p.flushHeredocs()
Expand All @@ -527,15 +530,15 @@ func (p *Printer) newlines(pos Pos) {
}

func (p *Printer) rightParen(pos Pos) {
if !p.minify {
if len(p.pendingHdocs) > 0 || !p.minify {
p.newlines(pos)
}
p.WriteByte(')')
p.wantSpace = spaceRequired
}

func (p *Printer) semiRsrv(s string, pos Pos) {
if p.wantsNewline(pos) {
if p.wantsNewline(pos, false) {
p.newlines(pos)
} else {
if !p.wroteSemi {
Expand Down Expand Up @@ -955,7 +958,7 @@ func (p *Printer) casePatternJoin(pats []*Word) {
if i > 0 {
p.spacedToken("|", Pos{})
}
if p.wantsNewline(w.Pos()) {
if p.wantsNewline(w.Pos(), true) {
if !anyNewline {
p.incLevel()
anyNewline = true
Expand Down Expand Up @@ -1015,7 +1018,7 @@ func (p *Printer) stmt(s *Stmt) {
}
p.incLevel()
for _, r := range s.Redirs[startRedirs:] {
if p.wantsNewline(r.OpPos) {
if p.wantsNewline(r.OpPos, true) {
p.bslashNewl()
}
if p.wantSpace == spaceRequired {
Expand Down Expand Up @@ -1413,7 +1416,7 @@ func (p *Printer) nestedStmts(stmts []*Stmt, last []Comment, closing Pos) {
func (p *Printer) assigns(assigns []*Assign) {
p.incLevel()
for _, a := range assigns {
if p.wantsNewline(a.Pos()) {
if p.wantsNewline(a.Pos(), true) {
p.bslashNewl()
} else {
p.spacePad(a.Pos())
Expand Down
17 changes: 13 additions & 4 deletions syntax/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,15 @@ func TestPrintMinify(t *testing.T) {
},
samePrint("foo >bar 2>baz <etc"),
samePrint("<<-EOF\n$(a|b)\nEOF"),
{
"a=$(\n\tcat <<'EOF'\n hello\nEOF\n)",
"a=$(cat <<'EOF'\n hello\nEOF\n)",
},
{
"(\n\tcat <<EOF\n hello\nEOF\n)",
"(cat <<EOF\n hello\nEOF\n)",
},
samePrint("diff -y <(cat <<EOF\n1\n2\n3\nEOF\n) <(cat <<EOF\n1\n4\n3\nEOF\n)"),
}
parser := NewParser(KeepComments(true))
printer := NewPrinter(Minify(true))
Expand Down Expand Up @@ -1161,8 +1170,8 @@ func TestPrintOptionsNotBroken(t *testing.T) {
{"SingleLine", []PrinterOption{SingleLine(true)}},
} {
printer := NewPrinter(opts.list...)
for i, tc := range append(fileTests, fileTestsNoPrint...) {
t.Run(fmt.Sprintf("File%s%03d", opts.name, i), func(t *testing.T) {
for _, tc := range append(fileTests, fileTestsNoPrint...) {
t.Run("File"+opts.name, func(t *testing.T) {
parser := parserPosix
if tc.Bats != nil {
parser = parserBats
Expand All @@ -1189,8 +1198,8 @@ func TestPrintOptionsNotBroken(t *testing.T) {
}
})
}
for i, tc := range printTests {
t.Run(fmt.Sprintf("Print%s%03d", opts.name, i), func(t *testing.T) {
for _, tc := range printTests {
t.Run("Print"+opts.name, func(t *testing.T) {
prog, err := parserBash.Parse(strings.NewReader(tc.in), "")
if err != nil {
t.Fatal(err)
Expand Down

0 comments on commit ca2088c

Please sign in to comment.