[PATCH v2] posix: update os.popen with subprocess (issue4746)

Matthieu Laneuville matthieu.laneuville at octobus.net
Tue Sep 5 09:52:44 EDT 2017


I don't have access to a windows machine to test a potential patch,
what do you suggest? I found a very similar function in windows.py so
the exact same modification there may work as well, but I can't test
it.

Regarding the name, is there a good naming convention to follow? Should
I use popen_wrapper() for instance?

Thank you for the review.
Matthieu

On Mon, 2017-09-04 at 22:23 +0900, Yuya Nishihara wrote:
> On Mon, 04 Sep 2017 14:22:36 +0200, matthieu.laneuville at octobus.net
> wrote:
> > # HG changeset patch
> > # User Matthieu Laneuville <matthieu.laneuville at octobus.net>
> > # Date 1504454402 -7200
> > #      Sun Sep 03 18:00:02 2017 +0200
> > # Node ID d892e64bf89c7f3e79039c5a2d1bd051b32eb686
> > # Parent  b2eb0aa445cbe7cadc8b7691c6266908adfc5057
> > # EXP-Topic issue4746
> > posix: update os.popen with subprocess (issue4746)
> > --- a/mercurial/posix.py	Tue Aug 22 21:21:43 2017 -0400
> > +++ b/mercurial/posix.py	Sun Sep 03 18:00:02 2017 +0200
> 
> Windows?
> 
> >  def popen(command, mode='r'):
> > -    return os.popen(command, mode)
> > +    if 'r' in mode:
> > +        return subprocess.Popen(command, shell=True,
> > stdout=subprocess.PIPE)
> > +    elif 'w' in mode:
> > +        return subprocess.Popen(command, shell=True,
> > stdin=subprocess.PIPE)
> > +    else:
> > +        raise ValueError
> 
> So this isn't popen(). It's probably better to rename the function.
> 
> http://mercurial.markmail.org/message/q5wpzw5g4lzmmbpb


More information about the Mercurial-devel mailing list