Git development
 help / color / mirror / Atom feed
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

  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