tweag / topiary

https://topiary.tweag.io/
MIT License
513 stars 17 forks source link

Errant blank lines added after runs of statements #203

Open Xophmeister opened 1 year ago

Xophmeister commented 1 year ago

Describe the bug New lines are inserted after "units of execution". After the last of which, in a run, this will be rendered as a blank line. This is particularly acute in indented sub-blocks.

To Reproduce Run the following through the Bash formatting rules:

while true; do
  command
done

Expected behavior The above code example should not be changed by the formatter. Currently, it is formatted as follows:

while true; do
  command

done

Environment

Additional context Not for v0.1

The Bash Tree Sitter grammar inserts anonymous \n nodes into the tree after items that should be followed by a new line (see discussion in #186). This is probably the best place to start looking as the root of the cause...

Xophmeister commented 1 year ago

(See https://github.com/tree-sitter/tree-sitter-bash/issues/155 for analysis)

patrislav1 commented 4 months ago

The Bash Tree Sitter grammar inserts anonymous \n nodes into the tree after items that should be followed by a new line (see discussion in #186). This is probably the best place to start looking as the root of the cause...

the spurious LF was apparently fixed half a year ago, is there a special reason why the bash grammar is pinned to an older revision?

torhovland commented 4 months ago

I think the reason we pinned some versions was to have all the grammars depend on the same version of Tree-sitter. It's possible this can all be reviewed and bumped by now.

Xophmeister commented 4 months ago

@patrislav1 the Bash Tree-sitter grammar has also changed significantly enough, in the meantime, that the formatting queries written for Bash are no longer valid with the newer grammar (i.e., it will immediately fail with syntax errors). In general, formatting queries need to be pinned to compatible grammars.

patrislav1 commented 4 months ago

Yep, I also found after un-pinning the old tree-sitter-bash revision and using the most recent one, topiary would abort with query errors.