| Submitter | Laurent Charignon |
|---|---|
| Date | June 5, 2015, 11:20 p.m. |
| Message ID | <08bb462f727b88812a46.1433546428@lcharignon-mbp.local> |
| Download | mbox | patch |
| Permalink | /patch/9536/ |
| State | Accepted |
| Headers | show |
Comments
On Fri, Jun 05, 2015 at 04:20:28PM -0700, Laurent Charignon wrote: > # HG changeset patch > # User Laurent Charignon <lcharignon@fb.com> > # Date 1433536278 25200 > # Fri Jun 05 13:31:18 2015 -0700 > # Node ID 08bb462f727b88812a46570b80c3a467c1ea23e9 > # Parent c39640d26a4c7546faef00b9e5c02af45ab8bf5e > record: exiting editor with non zero status should not stop recording session queued, thanks I've got one note below that might be worth doing in a followup - maybe ask other facebook folks how they feel about it? > > Before this patch, exiting a hunk edition in record with a non-zero status lead > to the end of the recording session losing previously selected hunks to record. > This patch introduces the more desirable behavior of warning the user and > continuing the recording session. > > diff --git a/mercurial/patch.py b/mercurial/patch.py > --- a/mercurial/patch.py > +++ b/mercurial/patch.py > @@ -1023,9 +1023,11 @@ > f.close() > # Start the editor and wait for it to complete > editor = ui.geteditor() > - ui.system("%s \"%s\"" % (editor, patchfn), > - environ={'HGUSER': ui.username()}, > - onerr=util.Abort, errprefix=_("edit failed")) > + ret = ui.system("%s \"%s\"" % (editor, patchfn), > + environ={'HGUSER': ui.username()}) > + if ret != 0: > + ui.warn(_("editor exited with exit code %d\n") % ret) > + continue > # Remove comment lines > patchfp = open(patchfn) > ncpatchfp = cStringIO.StringIO() > diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t > --- a/tests/test-commit-interactive.t > +++ b/tests/test-commit-interactive.t > @@ -1291,6 +1291,33 @@ > abort: error parsing patch: unhandled transition: range -> range > [255] > > +Exiting editor with status 1, ignores the edit but does not stop the recording > +session > + > + $ HGEDITOR=false hg commit -i <<EOF > + > y > + > e > + > n > + > EOF > + diff --git a/editedfile b/editedfile > + 1 hunks, 3 lines changed > + examine changes to 'editedfile'? [Ynesfdaq?] y > + > + @@ -1,3 +1,3 @@ > + -This is the first line > + -This change will be committed > + -This is the third line > + +This change will not be committed > + +This is the second line > + +This line has been added > + record this change to 'editedfile'? [Ynesfdaq?] e > + > + editor exited with exit code 1 > + record this change to 'editedfile'? [Ynesfdaq?] n > + > + no changes to record it occurs to me that the editor might leave a bunch of spew on the terminal and we should redisplay the diff? > + > + > random text in random positions is still an error > > $ cat > editor.sh << '__EOF__' > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel
> On Jun 8, 2015, at 7:45 AM, Augie Fackler <raf@durin42.com> wrote: > > On Fri, Jun 05, 2015 at 04:20:28PM -0700, Laurent Charignon wrote: >> # HG changeset patch >> # User Laurent Charignon <lcharignon@fb.com> >> # Date 1433536278 25200 >> # Fri Jun 05 13:31:18 2015 -0700 >> # Node ID 08bb462f727b88812a46570b80c3a467c1ea23e9 >> # Parent c39640d26a4c7546faef00b9e5c02af45ab8bf5e >> record: exiting editor with non zero status should not stop recording session > > queued, thanks > > I've got one note below that might be worth doing in a followup - > maybe ask other facebook folks how they feel about it? > >> >> Before this patch, exiting a hunk edition in record with a non-zero status lead >> to the end of the recording session losing previously selected hunks to record. >> This patch introduces the more desirable behavior of warning the user and >> continuing the recording session. >> >> diff --git a/mercurial/patch.py b/mercurial/patch.py >> --- a/mercurial/patch.py >> +++ b/mercurial/patch.py >> @@ -1023,9 +1023,11 @@ >> f.close() >> # Start the editor and wait for it to complete >> editor = ui.geteditor() >> - ui.system("%s \"%s\"" % (editor, patchfn), >> - environ={'HGUSER': ui.username()}, >> - onerr=util.Abort, errprefix=_("edit failed")) >> + ret = ui.system("%s \"%s\"" % (editor, patchfn), >> + environ={'HGUSER': ui.username()}) >> + if ret != 0: >> + ui.warn(_("editor exited with exit code %d\n") % ret) >> + continue >> # Remove comment lines >> patchfp = open(patchfn) >> ncpatchfp = cStringIO.StringIO() >> diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t >> --- a/tests/test-commit-interactive.t >> +++ b/tests/test-commit-interactive.t >> @@ -1291,6 +1291,33 @@ >> abort: error parsing patch: unhandled transition: range -> range >> [255] >> >> +Exiting editor with status 1, ignores the edit but does not stop the recording >> +session >> + >> + $ HGEDITOR=false hg commit -i <<EOF >> + > y >> + > e >> + > n >> + > EOF >> + diff --git a/editedfile b/editedfile >> + 1 hunks, 3 lines changed >> + examine changes to 'editedfile'? [Ynesfdaq?] y >> + >> + @@ -1,3 +1,3 @@ >> + -This is the first line >> + -This change will be committed >> + -This is the third line >> + +This change will not be committed >> + +This is the second line >> + +This line has been added >> + record this change to 'editedfile'? [Ynesfdaq?] e >> + >> + editor exited with exit code 1 >> + record this change to 'editedfile'? [Ynesfdaq?] n >> + >> + no changes to record > > it occurs to me that the editor might leave a bunch of spew on the > terminal and we should redisplay the diff? That's a great idea, I will put that in my queue > >> + >> + >> random text in random positions is still an error >> >> $ cat > editor.sh << '__EOF__' >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@selenic.com >> https://selenic.com/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/patch.py b/mercurial/patch.py --- a/mercurial/patch.py +++ b/mercurial/patch.py @@ -1023,9 +1023,11 @@ f.close() # Start the editor and wait for it to complete editor = ui.geteditor() - ui.system("%s \"%s\"" % (editor, patchfn), - environ={'HGUSER': ui.username()}, - onerr=util.Abort, errprefix=_("edit failed")) + ret = ui.system("%s \"%s\"" % (editor, patchfn), + environ={'HGUSER': ui.username()}) + if ret != 0: + ui.warn(_("editor exited with exit code %d\n") % ret) + continue # Remove comment lines patchfp = open(patchfn) ncpatchfp = cStringIO.StringIO() diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t --- a/tests/test-commit-interactive.t +++ b/tests/test-commit-interactive.t @@ -1291,6 +1291,33 @@ abort: error parsing patch: unhandled transition: range -> range [255] +Exiting editor with status 1, ignores the edit but does not stop the recording +session + + $ HGEDITOR=false hg commit -i <<EOF + > y + > e + > n + > EOF + diff --git a/editedfile b/editedfile + 1 hunks, 3 lines changed + examine changes to 'editedfile'? [Ynesfdaq?] y + + @@ -1,3 +1,3 @@ + -This is the first line + -This change will be committed + -This is the third line + +This change will not be committed + +This is the second line + +This line has been added + record this change to 'editedfile'? [Ynesfdaq?] e + + editor exited with exit code 1 + record this change to 'editedfile'? [Ynesfdaq?] n + + no changes to record + + random text in random positions is still an error $ cat > editor.sh << '__EOF__'