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

Support sequence of formatters #77

Open
zimbatm opened this issue Feb 27, 2021 · 11 comments
Open

Support sequence of formatters #77

zimbatm opened this issue Feb 27, 2021 · 11 comments
Labels
enhancement New feature or request
Milestone

Comments

@zimbatm
Copy link
Member

zimbatm commented Feb 27, 2021

In some cases, developers might want to run a sequence of formatters for the same file type. For example, re-order the import headers with one tool, and then use the main code formatted to fix the output of the first tool.

@zimbatm zimbatm added this to the Release 0.2 milestone Feb 27, 2021
@zimbatm zimbatm added the enhancement New feature or request label Mar 5, 2021
@zimbatm zimbatm modified the milestones: Release 0.2, Release 0.3 Apr 10, 2021
@zimbatm
Copy link
Member Author

zimbatm commented Apr 26, 2021

What happens is that some developers want to:

  1. Format the file imports in the file header with one tool
  2. Then run the normal formatter to fix the output of the first tool and make sure that the rest of the file is also properly formatted.

In order to support that, we need to find a way to apply these tools in sequence.

Maybe a config format change would be:

[formatter.ormolu]
pre_command = "import-fixer"
command = "ormolu"
after = "haskell-header"
includes = ["*.hs"]

@blaggacao
Copy link
Contributor

blaggacao commented Dec 12, 2021

I think the usage of sh is also a fair game, so that would mean nothing to do:

[formatter.beancount]
command = "sh"
options = [
  "-c",
  "for f in $@; do bean-format $f; done",
]
includes = ["*.beancount"]

Maybe a slightly friendlier sh interface might be a good & cheap option?

[formatter.beancount]
script = """
for f in $@; do bean-format $f; done
"""
includes = ["*.beancount"]

But I probably miss some aspects...

@zimbatm
Copy link
Member Author

zimbatm commented Dec 12, 2021

That's a good idea.

If there is tool A and B, we want to encourage calling A "$@" && B "$@" over for f in "#@"; do A "$f" && B "$f"; done. I learned that with terraform fmt that only supports one file at a time. The latter is easily one or two order of magnitude slower.

@blaggacao
Copy link
Contributor

Do you mean to implement a requires_singleton_file = true (or similar arg) ?

@zimbatm
Copy link
Member Author

zimbatm commented Dec 15, 2021

I meant that the example for sequencial calls should avoid using a for loop.

Overall, I would rather fix upstream formatters than add more config options :)

@blaggacao
Copy link
Contributor

Might this issue be even solvable via documentation, clarifying the use of sh or is there a need for more individual error reporting on which of the sequence of commands has failed?

@zimbatm
Copy link
Member Author

zimbatm commented Dec 17, 2021

Of course, Documentation over existing best practices is always a good first step.

@fbergroth
Copy link

fbergroth commented Dec 17, 2021

Hi, I just installed this (0.3.0), and I tried the following

[formatter.python]
command = "sh"
options = [
  "-c",
  """
  isort "$@" && black "$@"
  """,
]
includes = ["*.py"]

but was wondering why my python file wasn't formatted.

Turns out that the first file is passed as $0, but $@ is from $1 and onwards. Notice xx is missing from output.

$ sh -c 'echo AA $@ BB' xx yy zz
AA yy zz BB

I fixed this by adding a dummy argument:

[formatter.python]
command = "sh"
options = [
  "-c",
  """
  isort "$@" && black "$@"
  """,
  "dummy",
]
includes = ["*.py"]

Does this not happen for you?

Edit: isort --overwrite-in-place "$@" seems to be required for --stdin to work.

zimbatm added a commit that referenced this issue Dec 18, 2021
Put a dummy "--" as the first argument, otherwise the first file is
always ignored.

See #77 (comment)
zimbatm added a commit that referenced this issue Dec 18, 2021
Put a dummy "--" as the first argument, otherwise the first file is
always ignored.

See #77 (comment)
@zimbatm
Copy link
Member Author

zimbatm commented Dec 18, 2021

Good catch. f440332 includes your fix, and replaces dummy with --. -- is generally a convention in tools to say that the next arguments will be passed along to a sub-program.

@fbergroth
Copy link

Cool. It's easy to miss, so something friendlier would be nice, as suggested above.

Perhaps

sh = "..."

can desugar to

command = "/bin/sh"
options = ["-euc", "...", "--"]

brianmcgee pushed a commit to brianmcgee/treefmt that referenced this issue Dec 9, 2022
Put a dummy "--" as the first argument, otherwise the first file is
always ignored.

See numtide#77 (comment)
@yajo
Copy link

yajo commented Apr 3, 2024

I think it would be easier for the devs to just have a priority option. All hooks with same priority should run in parallel. Then, the same with next priority. Default priority could be 0. Example:

[formatter.isort]
command = "isort"
includes = ["*.py"]

[formatter.black]
command = "black"
priority = 1 # Runs after all formatters with priority 0 (default)
includes = ["*.py"]

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

No branches or pull requests

4 participants