tweag / topiary

https://topiary.tweag.io/
MIT License
579 stars 29 forks source link

Any workarounds for `@allow_blank_lines_before` #783

Closed blindFS closed 4 weeks ago

blindFS commented 1 month ago

I noticed that, maybe for now, @allow_blank_line_before has some issues.

image

And when I tried to create a formatter for nushell, @allow_blank_line_before does not work indeed.

For example, in following comments, a single "\n" node in between may correspond to multiple new-lines.

image

I have these in my query scheme file

[
  (comment)
] @allow_blank_line_before

[
  "\n"
] @append_hardline

Is there any trick to keep the empty lines in my case above?

Xophmeister commented 1 month ago

Thanks for this, @blindFS

Firstly, note that it's @allow_blank_line_before, rather than @allow_blank_lines_before. I suspect, however, that this is just a typo in your issue; I think Topiary would have complained about an unknown capture name...however, just to make sure.

That said, the behaviour of @allow_blank_line_before and vertical whitespace in general is not very intuitive. This is particularly true when you grammar produces \n anonymous nodes. I believe what you're observing is expected, but it probably shouldn't be and we should look into this further as using vertical whitespace is pretty common between comments...

nbacquey commented 1 month ago

Hello @blindFS

I tried to format a few comments with the tree-sitter grammar you linked, and the following seems to work:

[
  (comment)
] @leaf

(comment) @append_hardline @allow_blank_line_before

At least, it formats the following piece of code as it is:

# FOO

# BAR
# BAZ
# QUX

Note that I don't see any \n in the CST, so maybe we're not working with the same version of the grammar (I used https://github.com/nushell/tree-sitter-nu/commit/082a7c7df7db460da6b280c9f902bf2b19a2f423, i.e. the current main)

You can check what I've done in https://github.com/tweag/topiary/tree/nb/nu_comments

blindFS commented 1 month ago

Hello @blindFS

I tried to format a few comments with the tree-sitter grammar you linked, and the following seems to work:

[
  (comment)
] @leaf

(comment) @append_hardline @allow_blank_line_before

At least, it formats the following piece of code as it is:

# FOO

# BAR
# BAZ
# QUX

Note that I don't see any \n in the CST, so maybe we're not working with the same version of the grammar (I used nushell/tree-sitter-nu@082a7c7, i.e. the current main)

You can check what I've done in https://github.com/tweag/topiary/tree/nb/nu_comments

@nbacquey Thanks for your efforts! I use the same commit here, and I believe there's some inconsistent behaviours in the tree-sitter-nu's grammar. Try this snippet:

const a = 1
# FOO

# BAR
# BAZ
# QUX

You'll see "\n" nodes, and I'm afraid your method will fail in this case.

nbacquey commented 1 month ago

Indeed, in that case there are explicit \n nodes. I think the problem lies in the grammar, as you suggest.

I don't know about the tree-sitter ecosystem in general, but for Topiary specifically we noticed that formatting was easier when the \n nodes were removed by the grammar

blindFS commented 1 month ago

@nbacquey Thanks again for your fast response, maybe I'll create a PR to tree-sitter-nu latter to make it easier for topiary. I have a related question here, say if I append new line for both "\n" and (comment) here. And I somehow try to stop duplications by @do_nothing:

[
  "\n"
] @append_hardline

(
  (comment) @append_hardline
  .
  "\n" @do_nothing
)

I think the expected behavior will be:

  1. append new line to comment which is not followed by "\n"
  2. append new line only once for comment followed by "\n"

But no append triggered for scenario 1. Do I misunderstand the query semantics here?

blindFS commented 1 month ago

@nbacquey Never mind, I got it working with:

[
  "\n"
] @append_hardline

(
  (comment) @append_hardline
  .
  "\n"* @do_nothing
)
nbacquey commented 1 month ago

@nbacquey Never mind, I got it working with:

[
  "\n"
] @append_hardline

(
  (comment) @append_hardline
  .
  "\n"* @do_nothing
)

That's what I was going to suggest. It also works with "\n"? @do_nothing

In general, @do_nothing should only be used on nodes that may not appear in the match (for instance, a node followed by ? or *, or a node in a disjunction). Otherwise, the whole query will just always do nothing.

nbacquey commented 4 weeks ago

I'm closing the issue, since the problem is upstream, with the nushell tree-sitter grammar.