From: Jeff King <peff@peff.net>
To: "Suraj N. Kurapati" <sunaku@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress
Date: Thu, 6 Nov 2008 03:42:36 -0500 [thread overview]
Message-ID: <20081106084230.GA4407@sigill.intra.peff.net> (raw)
In-Reply-To: <200811050959.25824.sunaku@gmail.com>
On Wed, Nov 05, 2008 at 09:59:25AM -0800, Suraj N. Kurapati wrote:
> Allows the user to answer 'Stage this hunk' questions with a
> single keypress, just like in Darcs. Previously, the user was
> forced to press the Return key after every choice they made.
> This quickly becomes tiring, burdensome work for the fingers.
I think this is a reasonable goal, but I have a few questions/concerns.
- There are three versions of your patch, but nobody has commented.
Clearly we can see what changed, but it is not clear what advantage
one patch has over the other. Care to elaborate?
- Term::ReadKey, while common, is not part of base perl. So I think
using it needs to be conditional, and on systems without it we can
degrade to the current behavior.
- There's no facility in your patch for restoring the terminal if we
break out of the loop in an unexpected way (e.g., via the user
hitting ^C).
- This only enhances one particular input, the patch loop. It is
probably worth being consistent and allowing these behavior for other
menus (though the numeric inputs are a bit trickier).
-Peff
next prev parent reply other threads:[~2008-11-06 8:44 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-05 6:15 [PATCH/RFC] git-add--interactive.perl: Answer questions with a single keypress Suraj N. Kurapati
2008-11-05 6:31 ` [PATCH/RFC v2] " Suraj N. Kurapati
2008-11-05 17:59 ` [PATCH/RFC v3] git add -i: " Suraj N. Kurapati
2008-11-06 8:42 ` Jeff King [this message]
2008-11-06 16:15 ` Suraj N. Kurapati
2008-11-06 17:06 ` Junio C Hamano
2008-11-07 19:19 ` Jeff King
2008-11-07 19:19 ` Jeff King
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20081106084230.GA4407@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=sunaku@gmail.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox