Skip to content

Commit

Permalink
syntax: avoid extra newlines when printing case clauses
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mvdan committed Dec 4, 2022
1 parent c95f74c commit fa48a22
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 25 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This release drops support for Go 1.17 and includes many features and fixes.
- Don't require peeking two bytes after `echo *` - [#835]
- Simplify `${name:-}` to the equivalent `${name-}` - [#849]
- Don't print trailing whitespaces on nested subshells - [#814]
- Don't print extra newlines in some case clauses - [#779]
- Allow escaped newlines before unquoted words again - [#873]
- Parse a redirections edge case without spaces - [#879]
- Give a helpful error when `<<<` is used in POSIX mode - [#881]
Expand Down Expand Up @@ -654,6 +655,7 @@ module in v3.

Initial release.

[#779]: https://github.com/mvdan/sh/issues/779
[#803]: https://github.com/mvdan/sh/issues/803
[#814]: https://github.com/mvdan/sh/issues/814
[#835]: https://github.com/mvdan/sh/issues/835
Expand Down
60 changes: 35 additions & 25 deletions syntax/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ func (p *Printer) semiOrNewl(s string, pos Pos) {
if !p.minify {
p.space()
}
p.line = pos.Line()
p.advanceLine(pos.Line())
}
p.WriteString(s)
p.wantSpace = spaceRequired
Expand Down Expand Up @@ -415,14 +415,19 @@ func (p *Printer) indent() {

// TODO(mvdan): add an indent call at the end of newline?

// newline prints one newline and advances p.line to pos.Line().
func (p *Printer) newline(pos Pos) {
p.flushHeredocs()
p.flushComments()
p.WriteByte('\n')
p.wantSpace = spaceWritten
p.wantNewline, p.mustNewline = false, false
if p.line < pos.Line() {
p.line++
p.advanceLine(pos.Line())
}

func (p *Printer) advanceLine(line uint) {
if p.line < line {
p.line = line
}
}

Expand Down Expand Up @@ -486,7 +491,7 @@ func (p *Printer) flushHeredocs() {
if r.Hdoc != nil {
// Overwrite p.line, since printing r.Word again can set
// p.line to the beginning of the heredoc again.
p.line = r.Hdoc.End().Line()
p.advanceLine(r.Hdoc.End().Line())
}
p.wantSpace = spaceNotRequired
}
Expand All @@ -495,6 +500,8 @@ func (p *Printer) flushHeredocs() {
p.mustNewline = true
}

// newline prints between zero and two newlines.
// If any newlines are printed, it advances p.line to pos.Line().
func (p *Printer) newlines(pos Pos) {
if p.firstLine && len(p.pendingComments) == 0 {
p.firstLine = false
Expand All @@ -503,14 +510,17 @@ func (p *Printer) newlines(pos Pos) {
if !p.wantsNewline(pos) {
return
}
p.newline(pos)
if pos.Line() > p.line {
if !p.minify {
// preserve single empty lines
p.WriteByte('\n')
}
p.line++
p.flushHeredocs()
p.flushComments()
p.WriteByte('\n')
p.wantSpace = spaceWritten
p.wantNewline, p.mustNewline = false, false

l := pos.Line()
if l > p.line+1 && !p.minify {
p.WriteByte('\n') // preserve single empty lines
}
p.advanceLine(l)
p.indent()
}

Expand Down Expand Up @@ -567,9 +577,7 @@ func (p *Printer) flushComments() {
p.space()
}
// don't go back one line, which may happen in some edge cases
if p.line < cline {
p.line = cline
}
p.advanceLine(cline)
p.WriteByte('#')
p.writeLit(strings.TrimRightFunc(c.Text, unicode.IsSpace))
p.wantNewline = true
Expand Down Expand Up @@ -617,7 +625,7 @@ func (p *Printer) wordParts(wps []WordPart, quoted bool) {
p.line++
}
p.wordPart(wp, next)
p.line = wp.End().Line()
p.advanceLine(wp.End().Line())
}
}

Expand All @@ -632,11 +640,11 @@ func (p *Printer) wordPart(wp, next WordPart) {
p.WriteByte('\'')
p.writeLit(x.Value)
p.WriteByte('\'')
p.line = x.End().Line()
p.advanceLine(x.End().Line())
case *DblQuoted:
p.dblQuoted(x)
case *CmdSubst:
p.line = x.Pos().Line()
p.advanceLine(x.Pos().Line())
switch {
case x.TempFile:
p.WriteString("${")
Expand Down Expand Up @@ -863,7 +871,7 @@ func (p *Printer) testExpr(expr TestExpr) {
}

func (p *Printer) testExprSameLine(expr TestExpr) {
p.line = expr.Pos().Line()
p.advanceLine(expr.Pos().Line())
switch x := expr.(type) {
case *Word:
p.word(x)
Expand Down Expand Up @@ -1046,7 +1054,7 @@ func (p *Printer) stmt(s *Stmt) {
}

func (p *Printer) command(cmd Command, redirs []*Redirect) (startRedirs int) {
p.line = cmd.Pos().Line()
p.advanceLine(cmd.Pos().Line())
p.spacePad(cmd.Pos())
switch x := cmd.(type) {
case *CallExpr:
Expand Down Expand Up @@ -1133,7 +1141,7 @@ func (p *Printer) command(cmd Command, redirs []*Redirect) (startRedirs int) {
if p.minify || p.singleLine || x.Y.Pos().Line() <= p.line {
// leave p.nestedBinary untouched
p.spacedToken(x.Op.String(), x.OpPos)
p.line = x.Y.Pos().Line()
p.advanceLine(x.Y.Pos().Line())
p.stmt(x.Y)
break
}
Expand All @@ -1156,12 +1164,12 @@ func (p *Printer) command(cmd Command, redirs []*Redirect) (startRedirs int) {
}
} else {
p.spacedToken(x.Op.String(), x.OpPos)
p.line = x.OpPos.Line()
p.advanceLine(x.OpPos.Line())
p.comments(x.Y.Comments...)
p.newline(Pos{})
p.indent()
}
p.line = x.Y.Pos().Line()
p.advanceLine(x.Y.Pos().Line())
_, p.nestedBinary = x.Y.Cmd.(*BinaryCmd)
p.stmt(x.Y)
if indent {
Expand All @@ -1182,13 +1190,14 @@ func (p *Printer) command(cmd Command, redirs []*Redirect) (startRedirs int) {
} else if !x.Parens || !p.minify {
p.space()
}
p.line = x.Body.Pos().Line()
p.advanceLine(x.Body.Pos().Line())
p.comments(x.Body.Comments...)
p.stmt(x.Body)
case *CaseClause:
p.WriteString("case ")
p.word(x.Word)
p.WriteString(" in")
p.advanceLine(x.In.Line())
p.wantSpace = spaceRequired
if p.swtCaseIndent {
p.incLevel()
Expand Down Expand Up @@ -1228,6 +1237,7 @@ func (p *Printer) command(cmd Command, redirs []*Redirect) (startRedirs int) {
p.wantNewline = true
}
p.spacedToken(ci.Op.String(), ci.OpPos)
p.advanceLine(ci.OpPos.Line())
// avoid ; directly after tokens like ;;
p.wroteSemi = true
}
Expand Down Expand Up @@ -1360,7 +1370,7 @@ func (p *Printer) stmtList(stmts []*Stmt, last []Comment) {
if p.mustNewline || !p.minify || p.wantSpace == spaceRequired {
p.newlines(pos)
}
p.line = pos.Line()
p.advanceLine(pos.Line())
p.comments(midComs...)
p.stmt(s)
p.comments(endComs...)
Expand Down Expand Up @@ -1420,7 +1430,7 @@ func (p *Printer) assigns(assigns []*Assign) {
// Ensure we don't use an escaped newline after '=',
// because that can result in indentation, thus
// splitting "foo=bar" into "foo= bar".
p.line = a.Value.Pos().Line()
p.advanceLine(a.Value.Pos().Line())
p.word(a.Value)
} else if a.Array != nil {
p.wantSpace = spaceNotRequired
Expand Down
8 changes: 8 additions & 0 deletions syntax/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,14 @@ var printTests = []printCase{
"case $i in\n1)\n#foo \t\n;;\nesac",
"case $i in\n1)\n\t#foo\n\t;;\nesac",
},
{
"case $i in\n1)\n\t;;\n\n2)\n\t;;\nesac",
"case $i in\n1) ;;\n\n2) ;;\nesac",
},
{
"case $i\nin\n1)\n\t;;\nesac",
"case $i in\n1) ;;\nesac",
},
samePrint("case $i in\n1)\n\ta\n\t#b\n\t;;\nesac"),
samePrint("case $i in\n1) foo() { bar; } ;;\nesac"),
samePrint("case $i in\n1) ;; #foo\nesac"),
Expand Down

0 comments on commit fa48a22

Please sign in to comment.