Re: [PATCH 1/2] run-command: teach 'run_hook' about alternate worktrees

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 12, 2018, 21:49 UTC
Message-ID: <CAPig+cSbtATV8daCsDxXTfGPgGh-qrvoVzfBSB+WP3EuiDXNxQ@mail.gmail.com>
In-Reply-To: <42C41062-D27D-4BB8-8D8D-9272D37FAE88@gmail.com>

On Mon, Feb 12, 2018 at 3:58 PM, Lars Schneider <larsxschneider@gmail.com> wrote:

Show 11 quoted lines

>> On 12 Feb 2018, at 04:15, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> +int run_hook_ve(const char *const *env, const char *name, va_list args)
>> +{
>> +     return run_hook_cd_ve(NULL, env, name, args);
>> +}
>
> I think we have only one more user for this function:
>         builtin/commit.c:       ret = run_hook_ve(hook_env.argv,name, args);
>
> Would it be an option to just use the new function signature
> everywhere and remove the wrapper? Or do we value the old interface?

I did note that there was only one existing caller and considered simply modifying run_hook_ve()'s signature to accept the new 'dir' argument. I don't feel strongly one way or the other.

However, as mentioned elsewhere[1], I'm still not convinced that this one-off case of needing to chdir() warrants adding these overloads to 'run-command' at all, so I'm strongly considering (and was considering even for v1) instead just running this hook manually in builtin/worktree.c.

[1]: https://public-inbox.org/git/CAPig+cQ6Tq3J=bS8ymDqiXqUvoUiP59T=FGZgMw2FOAx0vyo=Q@mail.gmail.com/