Re: [PATCH 8/9] hook: teach 'hookcmd' config to alias hook scripts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > I would also need twice as many lines to run a script I wrote as a hook
> > - that is, the base case which is probably all most people will need. So
> > with your proposal I *must* name every hook I want to add. Instead of
> > "hook.pre-commit.command = ~/check-for-debug-strings" I need to
> > configure "hook.check-debug-strings.event = pre-commit" and
> > "hook.check-debug-strings.command = ~/check-for-debug-strings". That's a
> > little frustrating, and if I never want to skip that check or move it
> > later or something, it will be extra overhead for no gain for me.
> 
> The gain is that "git hook list" becomes a trivial "git config
> -show-origin --show-scope --get-regexp" wrapper.

I think that in both schemes, "git hook list" is not difficult to
implement.

> > I do agree that your approach bans the regrettably awkward
> > "hookcmd.~/check-for-debug-strings.skip = true", but I'm not sure
> > whether or not it's worth it.
> 
> That design choice also means that you can't expand the path using "git
> config --get --type=path.

I'm not sure what the use of this is, especially when the more important
position is in "hook.pre-commit.command" (which can be expanded, I
believe).

> As noted in the follow-up reply while we don't case normalize the LeVeL"
> part of "ThReE.LeVeL.KeY" that's tolower(), which we know isn't a 1=1
> mapping on some
> FS's. https://lore.kernel.org/git/87y2ebo16v.fsf@xxxxxxxxxxxxxxxxxxx/

We're comparing strings in the config file, though, not strings against
what's on the filesystem.

> To reply to all the above, yes, your suggestion comes out ahead in being
> less verbose.
> 
> But isn't the real difference not in the differing prefixes, i.e. hook.*
> and hookcmd.* (A) v.s. always hook.* (B, which is what I'm mainly
> focusing on, i.e. what requires the added complexity.

OK, this is a good point - it may be confusing for the user to remember
the difference between "hook" and "hookcmd", and your scheme eliminates
that.

> But that in that in your proposed way of doing it it's:
> 
>     hook.<event-or-name>.*
> 
> V.s. my suggestion of:
> 
>     hook.<name>.*

[snip discussion of how hook.<event-or-name> is bad]

I think you were thinking that someone would say "let's unify 'hook' and
'hookcmd' then" in response to you saying "'hook' vs 'hookcmd' is bad",
and you're arguing against this new point. I don't think anyone is
saying that, though.

> That complexity being that you use <event-or-name> and I use <name>, and
> you want to in turn support N number of "*.command" for any
> "hook.<event-or-name>".

I don't know if anyone is wanting to support this.

> The way to skip hooks in my proposal is:
> 
>     hook.<name>.command = true
> 
> Or whatever noop command you want on the RHS. In practice we wouldn't
> invoke "true", just like we don't invoke "cat" for "PAGER=cat".
> 
> But unlike "*.skip" that doesn't require complexity in implementation or
> user understanding, it just falls naturally out of the rest of the
> model.

This is a good point.

> Even with I think it's fair to say deep knowledge of your proposal at
> this point I still needed to read this a few times to see if that:
> 
>     command = reusable-hook
> 
> Is referring to:
> 
>     [hookcmd "reusable-hook"]
> 
> I.e. is it going to run:
> 
>     command = /long/path/reusable-hook
> 
> Or is it just re-specifying /long/path/reusable-hook but relying on a
> $PATH lookup?

This is true - seeing "hook.post-commit.command = reusable-hook", we
need to look at the rest of the config to see how it is interpreted.

Going back to the central idea, though, I think that the main advantage
of the scheme Emily proposed is the ability to write, more concisely:

  [hook.pre-commit]
  command = /path/to/command-1
  command = /path/to/command-2

instead of

  [hook.command-1]
  event = pre-commit
  command = /path/to/command-1
  [hook.command-2]
  event = pre-commit
  command = /path/to/command-2

But Ævar's scheme gives us the advantage that if need to do anything
more complicated (even merely slightly more complicated - for example,
skipping a hook or overriding the command of a hook), it would be hard
to write (and even to just figure it out) in Emily's scheme. In Ævar's
scheme, just following the standard "last config wins" rule (and knowing
about /usr/bin/true) can already do a lot. For this reason, I think it's
worth considering Ævar's scheme - writing 3 lines instead of 2 is not
much more difficult to teach and to do.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux