Why not `ui.argv`?, OOC?
On Mon, Mar 20, 2017 at 9:16 AM, Jun Wu <qu...@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 <phil...@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@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
