From: Junio C Hamano <gitster@pobox.com> To: "Orgad Shaneh via GitGitGadget" <gitgitgadget@gmail.com> Cc: git@vger.kernel.org, "Orgad Shaneh" <orgads@gmail.com>, "Eric Sunshine" <sunshine@sunshineco.com>, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com> Subject: Re: [PATCH v5 2/2] hooks: allow input from stdin for commit-related hooks Date: Wed, 09 Dec 2020 14:37:05 -0800 Message-ID: <xmqq360e1u9a.fsf@gitster.c.googlers.com> (raw) In-Reply-To: <25db4da3cd5fc7e81141078261086c392541c5d1.1607544408.git.gitgitgadget@gmail.com> (Orgad Shaneh via GitGitGadget's message of "Wed, 09 Dec 2020 20:06:48 +0000") "Orgad Shaneh via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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. > 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.
prev parent reply other threads:[~2020-12-09 22:42 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-17 15:02 [PATCH] hooks: allow input from stdin Orgad Shaneh via GitGitGadget 2020-11-17 19:59 ` Junio C Hamano 2020-11-19 15:50 ` [PATCH v2] " Orgad Shaneh via GitGitGadget 2020-11-19 15:56 ` [PATCH v3] hooks: allow input from stdin for commit-related hooks Orgad Shaneh via GitGitGadget 2020-11-19 19:16 ` Junio C Hamano 2020-11-19 20:41 ` Orgad Shaneh 2020-11-19 20:56 ` [PATCH v4 0/2] " Orgad Shaneh via GitGitGadget 2020-11-19 20:56 ` [PATCH v4 1/2] " Orgad Shaneh via GitGitGadget 2020-11-19 21:23 ` Eric Sunshine 2020-11-19 21:32 ` Junio C Hamano 2020-11-20 5:23 ` Orgad Shaneh 2020-11-20 6:38 ` Eric Sunshine 2020-11-20 6:48 ` Eric Sunshine 2020-11-20 7:16 ` Orgad Shaneh 2020-11-20 18:13 ` Junio C Hamano 2020-11-20 10:59 ` Ævar Arnfjörð Bjarmason 2020-11-20 12:34 ` Orgad Shaneh 2020-11-19 20:56 ` [PATCH v4 2/2] commit: fix stdin conflict between message and hook Orgad Shaneh via GitGitGadget 2020-12-09 20:06 ` [PATCH v5 0/2] hooks: allow input from stdin for commit-related hooks Orgad Shaneh via GitGitGadget 2020-12-09 20:06 ` [PATCH v5 1/2] hooks: lay foundations for passing stdin to hooks Orgad Shaneh via GitGitGadget 2020-12-09 20:06 ` [PATCH v5 2/2] hooks: allow input from stdin for commit-related hooks Orgad Shaneh via GitGitGadget 2020-12-09 22:37 ` Junio C Hamano [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=xmqq360e1u9a.fsf@gitster.c.googlers.com \ --to=gitster@pobox.com \ --cc=avarab@gmail.com \ --cc=git@vger.kernel.org \ --cc=gitgitgadget@gmail.com \ --cc=orgads@gmail.com \ --cc=sunshine@sunshineco.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git