On 14/04/2020 21:32, Jeff King wrote: > On Tue, Apr 14, 2020 at 04:15:11PM +0100, Phillip Wood wrote: > >> On 14/04/2020 01:54, Emily Shaffer wrote: >>> Not much to look at compared to the original RFC I sent some months ago. >>> This implements Peff's suggestion of using the "hookcmd" section as a >>> layer of indirection. >> >> I'm not really clear what the advantage of this indirection is. It seems >> unlikely to me that different hooks will share exactly the same command line >> or other options. In the 'git secrets' example earlier in this thread each >> hook needs to use a different command line. In general a command cannot tell >> which hook it is being invoked as without a flag of some kind. (In some >> cases it can use the number of arguments if that is different for each hook >> that it handles but that is not true in general) >> >> Without the redirection one could have >> hook.pre-commit.linter.command = my-command >> hook.pre-commit.check-whitespace.command = 'git diff --check --cached' >> >> and other keys can be added for ordering etc. e.g. >> hook.pre-commit.linter.before = check-whitespace >> >> With the indirection one needs to set >> hook.pre-commit.command = linter >> hook.pre-commit.check-whitespace = 'git diff --check --cached' >> hookcmd.linter.command = my-command >> hookcmd.linter.pre-commit-before = check-whitespace > > In the proposal I gave, you could do: > > hook.pre-commit.command = my-command > hook.pre-commit.command = git diff --check --cached > > If you want to refer to commands in ordering options (like your > "before"), then you'd have to refer to their names. For "my-command" > that's not too bad. For the longer one, it's a bit awkward. You _could_ > do: > > hookcmd.my-command.before = git diff --check --cached > > which is the same number of lines as yours. But I'd probably give it a > name, like: > > hookcmd.check-whitespace.command = git diff --check --cached > hookcmd.my-command.before = check-whitespace > > That's one more line than yours, but I think it separates the concerns > more clearly. And it extends naturally to more options specific to > check-whitespace. I agree that using a name rather than the command line makes things clearer here Best Wishes Phillip > -Peff >
