Patchwork crecord: fix issue when backgrounding editor would leave artefact

login
register
mail settings
Submitter lc2817@columbia.edu
Date July 16, 2015, 6:53 p.m.
Message ID <aacc79aaf710f69c3e06.1437072807@75.1.168.192.in-addr.arpa>
Download mbox | patch
Permalink /patch/10022/
State Superseded
Commit 2cccaf937a7a37de68f50c07cd7190cb53cf947d
Headers show

Comments

lc2817@columbia.edu - July 16, 2015, 6:53 p.m.
# HG changeset patch
# User Laurent Charignon <l.charignon@gmail.com>
# Date 1437017963 25200
#      Wed Jul 15 20:39:23 2015 -0700
# Node ID aacc79aaf710f69c3e060920bcf3255d88640a87
# Parent  35fa7c77c754aa4d156c42abfdb61ca178468872
crecord: fix issue when backgrounding editor would leave artefact

Before this patch:
- if a user was entering a commit message after having ran the curses
interface
- and then uses ctrl-z, followed by fg to put the editor in the
background/foreground
- then the curses interface would leave artefact on the screen of
the editor, making entering the commit message a difficult task

This happened because ncurses registers a signal handler for SIGTSTP and
does not restore the original signal handler after running.
More info at:
http://stackoverflow.com/questions/31440392/
curses-wrapper-messing-up-terminal-after-background-foreground-sequence/
31441709#31441709

This patch restores the original value of the signal handler after
running the curses interface and therefore fixes this issue.
It don't know how to add a test for this issue, I tested the scenario
above manually and it works correctly with the patch.
Laurent Charignon - July 16, 2015, 7:59 p.m.
This one should have the right whitespaces. 
Let me know if you have an issue and I will resend it.

Thanks,

> On Jul 16, 2015, at 11:53 AM, lc2817@columbia.edu wrote:
> 
> # HG changeset patch
> # User Laurent Charignon <l.charignon@gmail.com>
> # Date 1437017963 25200
> #      Wed Jul 15 20:39:23 2015 -0700
> # Node ID aacc79aaf710f69c3e060920bcf3255d88640a87
> # Parent  35fa7c77c754aa4d156c42abfdb61ca178468872
> crecord: fix issue when backgrounding editor would leave artefact
> 
> Before this patch:
> - if a user was entering a commit message after having ran the curses
> interface
> - and then uses ctrl-z, followed by fg to put the editor in the
> background/foreground
> - then the curses interface would leave artefact on the screen of
> the editor, making entering the commit message a difficult task
> 
> This happened because ncurses registers a signal handler for SIGTSTP and
> does not restore the original signal handler after running.
> More info at:
> https://urldefense.proofpoint.com/v1/url?u=http://stackoverflow.com/questions/31440392/&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=ovX3wDZmWBc057vX%2Bdkir5laRYh0DpiaFZ0DBnnb7ps%3D%0A&m=OCE9afx62pea7VZjYG1bpacqkMoTJeLGCxE2fMkZk38%3D%0A&s=c7f2e521ce6bc776bf01567dc9c39a8231aa344507b8297f28acc59907885350
> curses-wrapper-messing-up-terminal-after-background-foreground-sequence/
> 31441709#31441709
> 
> This patch restores the original value of the signal handler after
> running the curses interface and therefore fixes this issue.
> It don't know how to add a test for this issue, I tested the scenario
> above manually and it works correctly with the patch.
> 
> diff -r 35fa7c77c754 -r aacc79aaf710 mercurial/crecord.py
> --- a/mercurial/crecord.py	Sun Jul 12 17:59:25 2015 +0900
> +++ b/mercurial/crecord.py	Wed Jul 15 20:39:23 2015 -0700
> @@ -482,7 +482,10 @@
>     """
>     ui.write(_('starting interactive selection\n'))
>     chunkselector = curseschunkselector(headerlist, ui)
> +    f = signal.getsignal(signal.SIGTSTP)
>     curses.wrapper(chunkselector.main)
> +    # ncurses does not restore signal handler for SIGTSTP
> +    signal.signal(signal.SIGTSTP, f)
> 
> def testdecorator(testfn, f):
>     def u(*args, **kwargs):
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - July 17, 2015, 6:33 p.m.
On Thu, 2015-07-16 at 11:53 -0700, lc2817@columbia.edu wrote:
> # HG changeset patch
> # User Laurent Charignon <l.charignon@gmail.com>
> # Date 1437017963 25200
> #      Wed Jul 15 20:39:23 2015 -0700
> # Node ID aacc79aaf710f69c3e060920bcf3255d88640a87
> # Parent  35fa7c77c754aa4d156c42abfdb61ca178468872
> crecord: fix issue when backgrounding editor would leave artefact

Queued for default, thanks.

Patch

diff -r 35fa7c77c754 -r aacc79aaf710 mercurial/crecord.py
--- a/mercurial/crecord.py	Sun Jul 12 17:59:25 2015 +0900
+++ b/mercurial/crecord.py	Wed Jul 15 20:39:23 2015 -0700
@@ -482,7 +482,10 @@ 
     """
     ui.write(_('starting interactive selection\n'))
     chunkselector = curseschunkselector(headerlist, ui)
+    f = signal.getsignal(signal.SIGTSTP)
     curses.wrapper(chunkselector.main)
+    # ncurses does not restore signal handler for SIGTSTP
+    signal.signal(signal.SIGTSTP, f)
 
 def testdecorator(testfn, f):
     def u(*args, **kwargs):