Re: [PATCH v5 2/2] hooks: allow input from stdin for commit-related hooks

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 9, 2020, 22:37 UTC
Message-ID: <xmqq360e1u9a.fsf@gitster.c.googlers.com>
In-Reply-To: <25db4da3cd5fc7e81141078261086c392541c5d1.1607544408.git.gitgitgadget@gmail.com>

"Orgad Shaneh via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 5 quoted lines

> It's important to note that the hook author should be aware that stdin
> is not always applicable. For example, when running from IDE. This can
> be checked by isatty on stdin. The hooks should handle cases of closed
> input, and possibly fall-back to GUI input, or have sane defaults with
> a message to the user on this case.

I think this point was already brought up in the review on previous rounds, but when the hook needs to check the standard input anyway, it probably is a better design to close and have the hook open tty if needed, isn't it? I do not recall I saw a satisfactory answer to that question.

Show 7 quoted lines

> Allow stdin only for commit-related hooks. Some of the other hooks pass
> their own input to the hook, so don't change them.
>
> Note: If pre-commit reads from stdin, and git commit is executed with
> -F - (read message from stdin), stdin cannot be passed to the hook,
> since it will consume it before reaching the point where it is read for
> the commit message.

It is unclear what that Note is trying to achieve. Is it describing a known-bug in this implementation (if so, we'd probably need to update the documentation to mention this known regression)? Is it describing a reason why certain part of patch was done in a certain way that is not described in this message (e.g. when -F option is in effect the standard input stream is closed when invoking a hook)?

Thanks.