Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple formatters when using --stdin #274

Closed
terlar opened this issue Mar 14, 2024 · 8 comments
Closed

Multiple formatters when using --stdin #274

terlar opened this issue Mar 14, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@terlar
Copy link

terlar commented Mar 14, 2024

Is your feature request related to a problem? Please describe.

I configured the formatter dos2unix which I essentially applied to all files in the project, then everything seemed to work fine, however in my editor I utilize treefmt --stdin FILENAME < BUFFER in order to have consistent formatting across the project without duplicate configuration. Recently I noticed the formatting stopped working and when I debugged this I got this:

$ treefmt --stdin test.nix < test.nix
[WARN ] multiple formatters matched the path. picking the first one
{a = 1; b = 2; c = 3;}

So it didn't format the file because dos2unix was the first one.

Describe the solution you'd like

Either a way to select which formatter has priority in this case, or I guess the best case would be to run all the formatters, just like it does when you apply via treefmt test.nix.

Describe alternatives you've considered

  • Not have multiple formatters touching the same files.
  • Format file directly via argument, however my editor formatter relies on stdin/stdout formatting.
@terlar terlar added the enhancement New feature or request label Mar 14, 2024
terlar added a commit to terlar/treefmt that referenced this issue Mar 14, 2024
@terlar
Copy link
Author

terlar commented Jun 1, 2024

@brianmcgee I see the new release with golang rewrite is around the corner. Will this be fixed in the new version? Since the code is totally new, I assume this bug won't persist. But just making sure :)

@brianmcgee
Copy link
Member

@terlar this shouldn't be a problem with 2.0 with the introduction of #301

@brianmcgee
Copy link
Member

@terlar, if you have an example config you can share, I'd be happy to add it to the tests to ensure we cover this specifically.

@terlar
Copy link
Author

terlar commented Jun 1, 2024

I just tested this on the main branch and it seems the --stdin feature is broken or it works differently than before. It does not format from stdin, but instead tries to use stdin as the filenames or format the file provided as filename (used to figure out which formatter to use).

$ cat temp-test/treefmt.toml
# One CLI to format the code tree - https://git.numtide.com/numtide/treefmt
[formatter.dos2unix]
command = "dos2unix"
includes = ["*.yaml"]
options = ["--keepdate"]

[formatter.yamlfmt]
command = "yamlfmt"
includes = ["*.yaml"]

$ cat --show-ends temp-test/dos2unix+yaml.yaml
hello: world^M$
^M$
list:^M$
  - one^M$
  - two^M$
      - three^M$

$ cat temp-test/dos2unix+yaml.yaml | nix shell nixpkgs#dos2unix nixpkgs#yamlfmt --command nix run
 . -- -C temp-test --stdin sample.yaml
treefmt: error: failed to generate change set: failed to walk path: lstat /home/terje/src/github.com/numtide/treefmt/temp-test/sample.yaml: no such file or directory

@brianmcgee
Copy link
Member

@terlar seems I misunderstood how this was supposed to work as I was porting things to Go. I'll review the functionality more closely and bring 2.x inline. I've created #310

@brianmcgee
Copy link
Member

@terlar #313 was merged, give main another go whenever you have a chance.

❯ cat test.toml 
# One CLI to format the code tree - https://git.numtide.com/numtide/treefmt
[formatter.dos2unix]
command = "dos2unix"
includes = ["*.yaml"]
options = ["--keepdate"]

[formatter.yamlfmt]
command = "yamlfmt"
includes = ["*.yaml"]%  

❯ cat --show-ends dos2unix+yaml.yaml
hello: world^M$
^M$
list:^M$
  - one^M$
  - two^M$
      - three^M$

❯ cat dos2unix+yaml.yaml | nix run .# -- --config-file test.toml --stdin sample.yaml
hello: world
list:
  - one
  - two - three
   
❯ cat dos2unix+yaml.yaml | nix run .# -- --config-file test.toml --stdin sample.yaml -v
INFO format | dos2unix: 1 files processed in 899.52µs
INFO format | yamlfmt: 1 files processed in 2.720321ms
hello: world
list:
  - one
  - two - three

Formatters are applied first in priority order (lower the number, higher the precedence) with the default being 0, and then in lexicographical order.

@terlar
Copy link
Author

terlar commented Jun 5, 2024

Thank you! Looks promising, I will have a look soon.

@terlar
Copy link
Author

terlar commented Jun 11, 2024

Yes, I can confirm this now works. So I will close this issue. Thanks for the quick fix.

@terlar terlar closed this as completed Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants