Patchwork [5,of,5] ui: use bytes IO and convert EOL manually in ui.editor()

login
register
mail settings
Submitter Yuya Nishihara
Date March 29, 2017, 2:46 p.m.
Message ID <81aa3738f703e4354b0f.1490798801@mimosa>
Download mbox | patch
Permalink /patch/19822/
State Accepted
Headers show

Comments

Yuya Nishihara - March 29, 2017, 2:46 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1490791418 -32400
#      Wed Mar 29 21:43:38 2017 +0900
# Node ID 81aa3738f703e4354b0f853e87548a139b129bdd
# Parent  c43fe15a4a4bec542b7411087a1ad0233cb5614f
ui: use bytes IO and convert EOL manually in ui.editor()

Text IO sucks on Python 3 as it must be a unicode stream. We could introduce
a wrapper that converts unicode back to bytes, but it wouldn't be simple to
handle offsets transparently from/to underlying IOBase API.

Fortunately, we don't need to process huge text files, so let's stick to
bytes IO and convert EOL in memory.
David Soria Parra - March 31, 2017, 4:52 p.m.
On Wed, Mar 29, 2017 at 11:46:41PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1490791418 -32400
> #      Wed Mar 29 21:43:38 2017 +0900
> # Node ID 81aa3738f703e4354b0f853e87548a139b129bdd
> # Parent  c43fe15a4a4bec542b7411087a1ad0233cb5614f
> ui: use bytes IO and convert EOL manually in ui.editor()
> 
I am not sure if we should convert line endings. In particular crecord is
calling ui.edit() with a patch. A patch that would have CRLF line endings would
be displayed wrong and if someone wants to add a lineending or remove it in the
process, we would wrongly convert it to LF on Windows machines.
Yuya Nishihara - April 1, 2017, 10 a.m.
On Fri, 31 Mar 2017 09:52:37 -0700, David Soria Parra wrote:
> On Wed, Mar 29, 2017 at 11:46:41PM +0900, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1490791418 -32400
> > #      Wed Mar 29 21:43:38 2017 +0900
> > # Node ID 81aa3738f703e4354b0f853e87548a139b129bdd
> > # Parent  c43fe15a4a4bec542b7411087a1ad0233cb5614f
> > ui: use bytes IO and convert EOL manually in ui.editor()
> > 
> I am not sure if we should convert line endings. In particular crecord is
> calling ui.edit() with a patch. A patch that would have CRLF line endings would
> be displayed wrong and if someone wants to add a lineending or remove it in the
> process, we would wrongly convert it to LF on Windows machines.

We need EOL conversion on Windows because of notepad.exe. For crecord, we
might want to add ui.edit(text=False) option.
David Soria Parra - April 1, 2017, 4:49 p.m.
On Sat, Apr 01, 2017 at 07:00:07PM +0900, Yuya Nishihara wrote:
> On Fri, 31 Mar 2017 09:52:37 -0700, David Soria Parra wrote:
> > On Wed, Mar 29, 2017 at 11:46:41PM +0900, Yuya Nishihara wrote:
> > > # HG changeset patch
> > > # User Yuya Nishihara <yuya@tcha.org>
> > > # Date 1490791418 -32400
> > > #      Wed Mar 29 21:43:38 2017 +0900
> > > # Node ID 81aa3738f703e4354b0f853e87548a139b129bdd
> > > # Parent  c43fe15a4a4bec542b7411087a1ad0233cb5614f
> > > ui: use bytes IO and convert EOL manually in ui.editor()
> > > 
> > I am not sure if we should convert line endings. In particular crecord is
> > calling ui.edit() with a patch. A patch that would have CRLF line endings would
> > be displayed wrong and if someone wants to add a lineending or remove it in the
> > process, we would wrongly convert it to LF on Windows machines.
> 
> We need EOL conversion on Windows because of notepad.exe. For crecord, we
> might want to add ui.edit(text=False) option.

That could work. I feel all user input either in the form of crecords
patch or comittemplates should be returned to the user as is. Internal strings,
such as the default comittemplate or similar things should be converted to the
platform specific. An ui.edit(..., raw=True) switch could work

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -1232,11 +1232,11 @@  class ui(object):
         if self.configbool('experimental', 'editortmpinhg'):
             rdir = repopath
         (fd, name) = tempfile.mkstemp(prefix='hg-' + extra['prefix'] + '-',
-                                      suffix=extra['suffix'], text=True,
+                                      suffix=extra['suffix'],
                                       dir=rdir)
         try:
-            f = os.fdopen(fd, pycompat.sysstr("w"))
-            f.write(encoding.strfromlocal(text))
+            f = os.fdopen(fd, r'wb')
+            f.write(util.tonativeeol(text))
             f.close()
 
             environ = {'HGUSER': user}
@@ -1258,8 +1258,8 @@  class ui(object):
                         onerr=error.Abort, errprefix=_("edit failed"),
                         blockedtag='editor')
 
-            f = open(name)
-            t = encoding.strtolocal(f.read())
+            f = open(name, r'rb')
+            t = util.fromnativeeol(f.read())
             f.close()
         finally:
             os.unlink(name)