[PATCH 1 of 3] dispatch: store args and command run on ui

Phillip Cohen phillcofb at gmail.com
Mon Mar 20 15:58:10 EDT 2017


Why not `ui.argv`?, OOC?

On Mon, Mar 20, 2017 at 9:16 AM, Jun Wu <quark at fb.com> wrote:

> Excerpts from Ryan McElroy's message of 2017-03-20 11:43:22 +0000:
> > I think Jun needs to weigh in on this since the ui object is something
> > he's wanted to make immutable for a while and this doesn't seem to help
> > move us in that direction.
>
> Immutable is for the config object. I'm +1 to add ui.argv, because
> "sys.argv" is unreliable in chg's case and my attempt to change "sys.argv"
> got rejected.
>
> >
> > On 3/20/17 7:49 AM, Phil Cohen wrote:
> > > # HG changeset patch
> > > # User Phil Cohen <phillco at fb.com>
> > > # Date 1489995201 25200
> > > #      Mon Mar 20 00:33:21 2017 -0700
> > > # Node ID 2c1d5a02ec533055f182b0cc1280107fbfb76206
> > > # Parent  291951ad070b3fa39dd1d83503aa1011a20d9a21
> > > dispatch: store args and command run on ui
> > >
> > > This could also be used to simplify cases where we wrap
> `dispatch.runcommand` in
> > > extensions to get this information (e.g. in resolve.py and keyword.py)
> >
> > This summary is a bit of a non-sequitur: you describe additional things
> > you can do, but you haven't yet described the main thing you intend to
> > do. Please take a small step back and describe how you intend to use
> > this at first, then include this "could also be used" statement.
> >
> > >
> > > diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> > > --- a/mercurial/dispatch.py
> > > +++ b/mercurial/dispatch.py
> > > @@ -830,6 +830,8 @@
> > >               ui.warn(_("warning: --repository ignored\n"))
> > >
> > >           msg = ' '.join(' ' in a and repr(a) or a for a in fullargs)
> >
> > It seems that we should do some kind of shell escaping here to be
> > "correct", but I don't know how important that is for this use-case (and
> > you didn't change it yourself, so feel free to ignore me).
>
> This is for ui.log / blackbox. It could be a separate patch.
>
> >
> > > +        ui.args = fullargs
> >
> > I'm afraid that a name like 'ui.args' will tempt others to start using
> > it to grab arguments out of, which would be very bad (TM). Instead, I'd
> > suggest naming this "argstring" or "fullargs" to disambiguate it.
>
> It's meant to be a replacement for sys.argv. "fullargs" looks good to me.
>
> >
> > > +        ui.commandname = cmd
> >
> > Personally, I'd prefer more "private" variables set through setter
> > functions rather than this set-public-vars-directly style, but I'm not
> > sure if the community is onboard with that style yet (if not, I'll keep
> > working on it :-).
>
> While it'd be nice if it's more private and read-only to normal code, I'm a
> bit concerned that we make code unnecessarily long. Maybe we can add some
> check-code rules to forbid other modules from modifying some ui fields
> (including underscore ones).
>
> >
> > >           ui.log("command", '%s\n', msg)
> > >           strcmdopt = pycompat.strkwargs(cmdoptions)
> > >           d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
> > > diff --git a/mercurial/ui.py b/mercurial/ui.py
> > > --- a/mercurial/ui.py
> > > +++ b/mercurial/ui.py
> > > @@ -160,6 +160,8 @@
> > >           self._colormode = None
> > >           self._terminfoparams = {}
> > >           self._styles = {}
> > > +        self.commandname = None
> > > +        self.args = None
> > >
> > >           if src:
> > >               self.fout = src.fout
> > >
> >
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170320/5d9e9287/attachment.html>


More information about the Mercurial-devel mailing list