Skip to content

chore: rename max-batch-size to violates-rule-1#679

Merged
brianmcgee merged 3 commits intonumtide:mainfrom
jfly:violates-rule-1
Apr 7, 2026
Merged

chore: rename max-batch-size to violates-rule-1#679
brianmcgee merged 3 commits intonumtide:mainfrom
jfly:violates-rule-1

Conversation

@jfly
Copy link
Copy Markdown
Collaborator

@jfly jfly commented Mar 19, 2026

max-batch-size feels a bit silly to me: it's something people are only ever going to set to 1 as a workaround for formatters that violate rule 1 of the formatter spec.

I say let's call a spade a spade and make this a boolean with a name that makes you feel bad for using it.

@MagicRB
Copy link
Copy Markdown
Member

MagicRB commented Mar 19, 2026

isn't there a max arglist size on Linux? if the paths to the files are long and there are 1024 of such paths, you might go over the limit no?

@jfly
Copy link
Copy Markdown
Collaborator Author

jfly commented Mar 19, 2026

@MagicRB, maybe, but that's an internal affair to treefmt itself, not something folks should be using this max-batch-size feature for.

@MagicRB
Copy link
Copy Markdown
Member

MagicRB commented Mar 19, 2026

Agreed, just pointing out that treefmt then needs to account for the max arglist size and dynamically adjust its internal max-batch-size

@jfly
Copy link
Copy Markdown
Collaborator Author

jfly commented Mar 19, 2026

@MagicRB, I think treefmt is already doing something "good enough": it never creates batches larger than 1024. Do you see a problem with that?

@MagicRB
Copy link
Copy Markdown
Member

MagicRB commented Mar 19, 2026

main@omen:~$ getconf ARG_MAX
2097152
main@omen:~$ echo $((2097152 / 1024))
2048

on my system, ARG_MAX is plenty big. But that may not be on all systems. ideally treefmt would do that exact computation.

@jfly
Copy link
Copy Markdown
Collaborator Author

jfly commented Mar 19, 2026

I'm not opposed to treefmt doing something more clever. I'd be inclined to wait to do it until somebody actually runs into an issue, though.

It doesn't change my opinions about this PR, though!

@zimbatm
Copy link
Copy Markdown
Member

zimbatm commented Mar 19, 2026

ARG_MAX is hard to calculate.

When looking at ARG_MAX/NCARGS, you have to consider the space comsumption by both argv[] and envp[] (arguments and environment).

https://www.in-ulm.de/~mascheck/various/argmax/

@brianmcgee
Copy link
Copy Markdown
Member

I feel like violates-rule-1 is a bit obtuse. Maybe something like serial or sequential?

As for the max args, as @zimbatm pointed out, it's not easy to calculate. I decided on 1024 as a finger in the air decent start which seems to be working so far.

@jfly
Copy link
Copy Markdown
Collaborator Author

jfly commented Mar 20, 2026

I feel like violates-rule-1 is a bit obtuse. Maybe something like serial or sequential?

Obtuse is kind of the goal here, or at least I want something that looks like a code smell (sort of like the dangerouslySetInnerHTML from react). I don't think serial or sequential do that. @zimbatm, any ideas?

@MattSturgeon
Copy link
Copy Markdown
Contributor

MattSturgeon commented Mar 21, 2026

Maybe referring to how rule 1 is violated would be better?

Rule 1 is:

Files passed as arguments

https://github.com/numtide/treefmt/blob/main/docs/site/reference/formatter-spec.md#L22

And more specifically, it means accepting multiple positional args as multiple files.

So something like no-positional-arg-support and/or no-muti-file-support could be more explicit about what is violated, while still hinting that this is not a good thing.

@jfly
Copy link
Copy Markdown
Collaborator Author

jfly commented Apr 7, 2026

@brianmcgee, I was hoping to land this before the next release so it wouldn't be a breaking change.

With 2.5.0 out, I'd suggest we either land this quickly, or abandon it. Thoughts?

@brianmcgee
Copy link
Copy Markdown
Member

My bad I'll merge and recut. I messed up the nixpkgs PR anyway

@brianmcgee brianmcgee force-pushed the violates-rule-1 branch 2 times, most recently from 96bba8e to a803a28 Compare April 7, 2026 19:29
@brianmcgee
Copy link
Copy Markdown
Member

Making an executive decision and going with Matt's suggestion of no-positional-arg-support. I think it's clearer than violates-rule-1 but also conveys that the formatter is deviating from the norm. I'll merge this after dinner and recut the 2.5.0 release before I fix the bad nixpkgs PR.

testing and others added 3 commits April 7, 2026 20:53
`max-batch-size` feels a bit silly to me: it's something people are only
ever going to set to 1 as a workaround for formatters that violate rule
1 of the formatter spec.

I say let's call a spade a spade and make this a boolean with a name
that makes you feel bad for using it.
Signed-off-by: Brian McGee <brian@bmcgee.ie>
I feel this is clearer than `violates-rule-1`.

Signed-off-by: Brian McGee <brian@bmcgee.ie>
@brianmcgee brianmcgee merged commit 7ab41ba into numtide:main Apr 7, 2026
5 checks passed
@jfly
Copy link
Copy Markdown
Collaborator Author

jfly commented Apr 8, 2026

Thanks!

@jfly jfly deleted the violates-rule-1 branch April 8, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants