remove leading and trailing empty lines in a block #311
Conversation
ensures that format removes empty lines at the top of a block unconditionally and bottom unless it is followed by an else-if statement. see issue mvdan#284
ensures that empty line before an else-if statement is not removed. see mvdan#284.
see mvdan#284. trailing empty lines are removed only if it is not followed by an else-if statement.
|
LGTM, what are we waiting on here? |
|
Thanks for the patch and apologies for the slow response. The code change looks fine. I also ran the updated gofumpt on a number of my projects and it mostly looks good, except in two cases. First, if-elses still remove the trailing empty line, because your code only opts out for if-elseif. I think that needs to be adjusted so that the empty line below is not removed: meta := s.MetadataGraph()
var rdeps map[PackageID]*metadata.Package
if transitive {
rdeps = meta.ReverseReflexiveTransitiveClosure(id)
// Remove the original package ID from the map.
// (Callers all want irreflexivity but it's easier
// to compute reflexively then subtract.)
delete(rdeps, id)
-
} else {
// direct reverse dependencies
rdeps = make(map[PackageID]*metadata.Package)
for _, rdepID := range meta.ImportedBy[id] {
if rdep := meta.Packages[rdepID]; rdep != nil {
rdeps[rdepID] = rdep
}
}
}Second, I wonder whether this change is a good idea or not: switch tok {
-
case token.FOR, token.IF:
if rhs {
expr = p.parseExpr()
break
}
comp, ident := p.parseComprehension()
if comp != nil {
return nil, nil, comp, false
}
expr = ident
case token.LET:
let, ident := p.parseLetDecl()
if let != nil {
return nil, nil, let, false
}
expr = identNote how empty lines are used to separate the switch cases, so a leading empty line does add some symettry. Given how common large switches with many cases are, perhaps we should always allow a leading empty line in a switch as a special case. I don't think it's worth getting into heuristics for them. |
This pull request is intended to achieve what was discussed on issue #284 leaving empty trailing lines in a block if it is followed by an else-if statement, otherwise removes it. Also takes over (somewhat) PR #286.
@mvdan please feel free to suggest any change you see suitable.
close #284