treefmt icon indicating copy to clipboard operation
treefmt copied to clipboard

Support sequence of formatters

Open zimbatm opened this issue 3 years ago • 11 comments

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 avatar Feb 27 '21 16:02 zimbatm

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"]

zimbatm avatar Apr 26 '21 09:04 zimbatm

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...

blaggacao avatar Dec 12 '21 04:12 blaggacao

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.

zimbatm avatar Dec 12 '21 15:12 zimbatm

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

blaggacao avatar Dec 15 '21 12:12 blaggacao

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 :)

zimbatm avatar Dec 15 '21 14:12 zimbatm

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?

blaggacao avatar Dec 17 '21 14:12 blaggacao

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

zimbatm avatar Dec 17 '21 16:12 zimbatm

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.

fbergroth avatar Dec 17 '21 21:12 fbergroth

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.

zimbatm avatar Dec 18 '21 11:12 zimbatm

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", "...", "--"]

fbergroth avatar Dec 18 '21 20:12 fbergroth

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"]

yajo avatar Apr 03 '24 07:04 yajo