From: "Ævar Arnfjörð Bjarmason" <firstname.lastname@example.org> To: Orgad Shaneh <email@example.com> Cc: Eric Sunshine <firstname.lastname@example.org>, Orgad Shaneh via GitGitGadget <email@example.com>, Git List <firstname.lastname@example.org> Subject: Re: [PATCH v4 1/2] hooks: allow input from stdin for commit-related hooks Date: Fri, 20 Nov 2020 11:59:22 +0100 Message-ID: <email@example.com> (raw) In-Reply-To: <CAGHpTBKHmdjqrz1ABdGUUz7AwcixU_VBy1DQzybpFizqVo8C7A@mail.gmail.com> On Fri, Nov 20 2020, Orgad Shaneh wrote: > On Thu, Nov 19, 2020 at 11:23 PM Eric Sunshine <firstname.lastname@example.org> wrote: >> >> On Thu, Nov 19, 2020 at 3:57 PM Orgad Shaneh via GitGitGadget >> <email@example.com> wrote: >> > Let hooks receive user input if applicable. >> > [...] >> > This allows for example prompting the user to choose an issue >> > in prepare-commit-msg, and add "Fixes #123" to the commit message. >> > >> > Another possible use-case is running sanity test on pre-commit, >> > and having a prompt like "This and that issue were found in your >> > changes. Are you sure you want to commit? [Y/N]". >> >> These use-cases really help readers understand the motivation for this >> change. Good. >> >> > 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), the message is not read >> > correctly. This is fixed in the follow-up commit. >> >> Rather than making such a fundamental change and having to deal with >> the fallout by introducing complexity to handle various special-cases >> which pop up now and in the future, I wonder if it makes more sense to >> instead just update documentation to tell hook authors to read >> explicitly from the console rather than expecting stdin to be >> available (since stdin may already be consumed for other purposes when >> dealing with hooks or commands which invoke the hooks). > > On the first revision I had several links in the commit message to > users who solved it this way. This solution however is not optimal. > I have a prepare-commit-msg hook that requires user interaction for > choosing an issue. This hook must work from the terminal and also > from GUI applications like IDE. > > Currently the hook always pops a GUI window, but when using it > from the terminal this is inconvenient (and when running over > remote SSH without X forwarding it can't work), so I'd like it to be > usable also from the terminal. > > To achieve that, I created 2 classes - one for terminal and one > for GUI, and trying to choose the correct class by checking if > stdin is a tty. The condition looks like this (Ruby): > client = STDIN.tty? ? Terminal.new : GUI.new > > At this point I was surprised to discover that Git closes stdin, > so the condition is never satisfied, and I always end up with GUI. > > As I mentioned, I need it to work also when executed from > GUI applications, so just reading from the console will not work > in my case. I tried other ways to detect "running from terminal" > without the tty condition, but couldn't. The environment variables > are identical when running in a GUI terminal and in the IDE. > > Can you suggest an alternative way to determine if I can accept user > input from the console or not? Like Eric noted in his reply I can't think of a way to do that particular thing reliably either, and agree with his comments that if such a way is found / some aspect of this change is kept having this explanation in the patch/commit message is really helpful. I think what you're trying to do here isn't a good fit for most git workflows. Instead of trying to interactively compose a commit message why not change the commit template to start with e.g.: # You must replace XXX with an issue number here!: Issue #XXX: That gives the user the same thing to fill out, but in their editor instead of via some terminal/GUI prompt. They need to write the rest of the commit message anyway in the editor, so even if you could why open up two UIs? Projects that have these conventions also typically settle on just not trying to solve this problem on the client-side, but e.g. having a pre-receive hook that does the validation, or do it via CI / before a merge to master happens etc.
next prev parent reply other threads:[~2020-11-20 11:01 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 [this message] 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
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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.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
firstname.lastname@example.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 \ email@example.com 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