[PATCH 1 of 3] dispatch: store args and command run on ui
Jun Wu
quark at fb.com
Mon Mar 20 12:16:27 EDT 2017
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
> >
>
More information about the Mercurial-devel
mailing list