On Sun, Nov 24, 2024 at 11:25:21AM +0900, Junio C Hamano wrote:
> I am wondering what we can do to add a different kind of checks to
> help file types with fixed format by extending the same mechanism,
> or the checks I have in mind are too different from the whitespace
> checks and shoehorning it into the existing mechanism does not make
> sense. The particular check I have an immediate need for is for a
> filetype with lines, each has exactly 4 fields separated with HT in
> between, so the check would ask "does each line have exactly 3 HT on
> it?" It would be extended to verify CSV files with fixed number of
> fields (but the validator needs to be aware of the quoting rules for
> comma in a value in fields).
Coming from a devil's advocate position: what makes these CSV format
checks any different than syntax checks we get from a compiler? Or for
that matter, the result of running "make test"?
I.e., why implement a complex system for single-line verification
plugins when you'd be left with the much larger problem of evaluating
whole-tree states. And once you have solutions for that (like using
branches to separate unverified work and then merging it once it has
passed checks), then simple things like line syntax are easy to call
there.
Now you could argue that the existing whitespace checks are similarly
redundant. Rather than having "apply" complain about whitespace errors,
you could just check them as part of "make test".
The reasons I can think of for doing something like this are:
- catching problems earlier is almost always less work for the user
- for things that _are_ line-oriented, looking at individual diff
lines lets you focus on problems being added, without worrying about
existing violations in the final state. OTOH, that's not foolproof;
if you modify a line with an existing whitespace problem without
fixing it, "diff --check" will still complain.
So I'm not necessarily against it. But it seems like a very deep rabbit
hole to start adding in shared-library line validators, because I think
it ends in "now compile this before I agree to apply the patch". And I
think Git's model has mostly been the opposite: make it cheap and
private to branch and make changes (including applying patches) so that
you can inspect the state before deciding whether and how to publish.
-Peff
