tweag / topiary

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

unable to use experimental language formatters #663

Open josharian opened 6 months ago

josharian commented 6 months ago

Describe the bug

I'm here kicking the tires, because I need to write a formatter and I love the general approach.

I followed the readme and built everything locally, but can't get any of the experimental language formatters to work. It rejects the language.

To Reproduce

Run:

$ topiary config | grep -C 3 tree
extensions = ["toml"]

[[language]]
name = "tree_sitter_query"
extensions = ["scm"]
$ topiary fmt -l tree_sitter_query -q /Users/josh/x/topiary/queries/tree-sitter-query.scm go.scm
error: invalid value 'tree_sitter_query' for '--language <LANGUAGE>'
  [possible values: json, nickel, ocaml, ocaml-interface, ocamllex, toml]

For more information, try '--help'.

Expected behavior

The language is in the config, and I provided a query file, so according to the readme, I think it ought to work?

Environment

Xophmeister commented 6 months ago

Thank you for bringing this to our attention. This is another divergence between the documentation and reality (e.g., see #652), I'm afraid. The --language flag only accepts "supported" languages, even though it would make sense for it to support all languages for which there is a grammar-query pair.

As a workaround, Topiary should format Tree-sitter query files using the query file topiary-queries/queries/tree-sitter-query.scm. Editing that file should enable you to tweak the formatting to your specification.

Otherwise, this is a bug that will (perhaps) be addressed by #643.

josharian commented 6 months ago

Thanks. I'm afraid that that workaround doesn't currently work:

$ topiary fmt -q /Users/josh/x/topiary/topiary-queries/queries/tree-sitter-query.scm go.scm 
error: The following required argument was not provided: language

Usage: topiary-cli [OPTIONS] <COMMAND>

For more information, try '--help'.

I patched in #643, also no luck:

$ topiary fmt -l tree_sitter_query -q /Users/josh/x/topiary/topiary-queries/queries/tree-sitter-query.scm queries/go.scm
error: the argument '--language <LANGUAGE>' cannot be used with '[FILES]...'

Usage: topiary format --query <QUERY> <--language <LANGUAGE>|FILES>

For more information, try '--help'.
$ topiary fmt -q /Users/josh/x/topiary/topiary-queries/queries/tree-sitter-query.scm queries/go.scm 
error: The following required argument was not provided: language

Usage: topiary-cli [OPTIONS] <COMMAND>

For more information, try '--help'.

I would also note, FWIW, that I found this bit: <--language <LANGUAGE>|FILES> pretty hard to parse. I would suggest <FILES | --language LANGUAGE> as easier on the eyes/brain.

Xophmeister commented 6 months ago

Thanks. I'm afraid that that workaround doesn't currently work:

$ topiary fmt -q /Users/josh/x/topiary/topiary-queries/queries/tree-sitter-query.scm go.scm 

You don't need to provide the --query argument if you edit the default query for the grammar. Topiary will simply pick up those changes transparently; i.e., topiary fmt go.scm should do what you expect. Apologies if this wasn't clear.

I patched in #643, also no luck:

643 is currently a draft PR; it's not complete yet, but the overall intention of the PR is to decouple the library and CLI, which is where the notion of "supported languages" originates. I therefore suspect there's a good chance that this oversight will eventually be corrected by this PR.

I would also note, FWIW, that I found this bit: <--language <LANGUAGE>|FILES> pretty hard to parse. I would suggest <FILES | --language LANGUAGE> as easier on the eyes/brain.

I agree. Unfortunately, this is generated automatically as part of the argument parsing library we use.

josharian commented 6 months ago

topiary fmt go.scm should do what you expect.

Hah. Thanks, that did it. Now to go muck around a bit under the hood...

Xophmeister commented 6 months ago

Reopening, as this is still a bug that needs fixing.