Re: [PATCH v4 3/9] hook: add list command
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Oct 6, 2020, 20:46 UTC
- Message-ID
- <20201006204608.GL331156@google.com>
- In-Reply-To
- <20200923230451.1279511-1-jonathantanmy@google.com>
On Wed, Sep 23, 2020 at 04:04:51PM -0700, Jonathan Tan wrote:
Show 10 quoted lines
> > > $ git hook list pre-commit > > ~/baz/from/hookcmd.sh > > ~/bar.sh > > In the tests below, there is a "local:" prefix (or similar). It's > clearer if the commit message has that too. > > Also, looking at a later commit, the "list" command probably should > include the legacy hook if it exists.
Have added it as a separate patch for v5, hopefully that will make more sense.
Show 15 quoted lines
>
> > +static void emplace_hook(struct list_head *pos, const char *command)
> > +{
> > + struct hook *to_add = malloc(sizeof(struct hook));
> > + to_add->origin = current_config_scope();
> > + strbuf_init(&to_add->command, 0);
> > + /* even with use_shell, run_command() needs quotes */
> > + strbuf_addf(&to_add->command, "'%s'", command);
> > +
> > + list_add_tail(&to_add->list, pos);
> > +}
>
> It might be odd to a programmer reading this that an existing "struct
> hook" with the same name is not reused - the scanning of the list done
> in hook_config_lookup() could probably go here instead.Sure, done.
Show 10 quoted lines
> > > +test_expect_success 'git hook list orders by config order' ' > > + setup_hooks && > > + > > + cat >expected <<-EOF && > > + global: $ROOT/path/def > > + local: $ROOT/path/ghi > > Will the "global" strings etc. be translated? If yes, it's probably not > worth it to align the paths in this way.
Asked more offline. Jonathan was saying that translation might result in scope name + tab character leaving the path in different columns depending on the scope anyways, so there's no point in using a tab character instead of a space character here. That seems reasonable; I'll switch.
- Emily