Re: Topics currently in the Stalled category

In-reply-to: <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org> (sfid-20121120_190548_379327_D3EE7D14)
References: <7vpq39up0m.fsf@alter.siamese.dyndns.org> <7vy5hvq1ey.fsf@alter.siamese.dyndns.org> <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org> (sfid-20121120_190548_379327_D3EE7D14)
Fcc: outbox
--------

junio c hamano wrote:
 > Here is a list of stalled topics I am having trouble deciding what
 > to do (the default is to dismiss them around feature freeze).
...
 > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 > 
 >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 >  session, not realizing git will receive the signal. Since most editors
 >  will take over the terminal and will block SIGINT, this is not likely
 >  to confuse anyone.
 > 
 >  Some people raised issues with emacsclient, which are addressed by this
 >  re-roll. It should probably also handle SIGQUIT, and there were a
 >  handful of other review comments.
 > 
 >  Anybody interested in moving this forward?

i started this, but then jeff took it and ran with it and made it right. i think the remaining changes are small -- if jeff would prefer, i can probably finish it. (but i won't guarantee not to mess up the "From:" lines. :-)

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 36.0 degrees)

On Tue, Nov 20, 2012 at 09:46:47PM -0500, Paul Fox wrote:

I'm also interested. I sometimes use ":r !command" in vim, so far I never needed to use Ctrl-C, but maybe in future.

The SIGINT part seems to be finished, we need to decide what about SIGQUIT.

Krzysiek

On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:

My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think there may have been one or two other minor comments in response to the series). I am on vacation this week, but can revisit it next week. If somebody wants to re-roll it in the meantime, that would be fine with me.

-Peff

[1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
    nearly as widely used as SIGINT, but it sounds more like the
    principle of least surprise to treat them the same.

jeff wrote:
 > On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:
 > 
 > > >  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 > > >  > 
 > > >  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 > > >  >  session, not realizing git will receive the signal. Since most editors
 > > >  >  will take over the terminal and will block SIGINT, this is not likely
 > > >  >  to confuse anyone.
 > > >  > 
 > > >  >  Some people raised issues with emacsclient, which are addressed by this
 > > >  >  re-roll. It should probably also handle SIGQUIT, and there were a
 > > >  >  handful of other review comments.
 > > >  > 
 > > >  >  Anybody interested in moving this forward?
 > > > 
 > > > i started this, but then jeff took it and ran with it and made it
 > > > right.  i think the remaining changes are small -- if jeff would
 > > > prefer, i can probably finish it.  (but i won't guarantee not to
 > > > mess up the "From:" lines.  :-)
 > > > 
 > > 
 > > I'm also interested. I sometimes use ":r !command" in vim, so far I never
 > > needed to use Ctrl-C, but maybe in future.
 > > 
 > > The SIGINT part seems to be finished, we need to decide what about SIGQUIT.
 > 
 > My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think
 > there may have been one or two other minor comments in response to the
 > series). I am on vacation this week, but can revisit it next week. If
 > somebody wants to re-roll it in the meantime, that would be fine with
 > me.
 > 
 > -Peff
 > 
 > [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
 >     nearly as widely used as SIGINT, but it sounds more like the
 >     principle of least surprise to treat them the same.

i see no real point in treating them the same -- as you suggest, one would only use SIGQUIT if SIGINT didn't work, and then you'd want them to be treated differently. so i'd be happy with the current code in that regard.

i think krzysiek said that since editors usually catch SIGQUIT, git should kill the editor when receiving SIGQUIT, essentially translating the SIGQUIT to SIGTERM for the editor. (please correct me if i misunderstood.) since well-behaved editors will die quickly anyway (they get EIO on their next read from stdin), i'm not sure there's a compelling reason for that extra step.

but i have no real objection to that behavior if others think it's right -- there's certainly logic in saying that if git dies it should ensure the editor does too.

(i'm away for the rest of the week also.)

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 44.6 degrees)

Jeff King <peff@peff.net> writes:

Sounds sensible. I wonder what happens when the editor is suspended ;-)

On Wed, Nov 21, 2012 at 11:53:04AM -0800, Junio C Hamano wrote:

I think we would want to leave SIGTSTP alone; the editor should typically respect it, and we would want to also pause until we get SIGCONT (although even if we did continue, we would just be blocking on wait() for the editor, anyway, so it is not a big deal).

Implicit in my "least surprise" comment above is that handling SIGQUIT would match what system(3) does, so it makes sense to me to match that (it also blocks SIGCHLD, but I do not think that really matters from a user-visible perspective).

-Peff