| Submitter | Gregory Szorc |
|---|---|
| Date | April 27, 2017, 11:08 p.m. |
| Message ID | <f495df1ca1cc3583ecfb.1493334529@ubuntu-vm-main> |
| Download | mbox | patch |
| Permalink | /patch/20306/ |
| State | Deferred |
| Headers | show |
Comments
This series is a "pick your poison" if you are a reviewer. I'm partial to the more complex one adding an argument to @command. But it may not be appropriate for stable. So a hacky fallback is provided as an alternative. On Thu, Apr 27, 2017 at 4:08 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1493333518 25200 > # Thu Apr 27 15:51:58 2017 -0700 > # Branch stable > # Node ID f495df1ca1cc3583ecfbed208de69a966eb74aea > # Parent f9dd22b588e9f0eb475ce522a78c640d35c12310 > tests: demonstrate `hg show` mapping to showconfig > > `hg config` has a "showconfig" alias. In a vanilla installation where > the "show" extension isn't enabled, `hg show` will map to > `hg showconfig`. Add a test demonstrating this confusing behavior. > > diff --git a/tests/test-show.t b/tests/test-show.t > --- a/tests/test-show.t > +++ b/tests/test-show.t > @@ -1,3 +1,20 @@ > +`hg show` without extension enabled says to enable the extension > +TODO this is broken due to matching with showconfig > + > + $ hg show > + defaults.backout=-d "0 0" > + defaults.commit=-d "0 0" > + defaults.shelve=--date "0 0" > + defaults.tag=-d "0 0" > + devel.all-warnings=true > + largefiles.usercache=$TESTTMP/.cache/largefiles > + ui.slash=True > + ui.interactive=False > + ui.mergemarkers=detailed > + ui.promptecho=True > + web.address=localhost > + web.ipv6=False > + > $ cat >> $HGRCPATH << EOF > > [extensions] > > show = >
On Thu, 27 Apr 2017 16:11:34 -0700, Gregory Szorc wrote: > This series is a "pick your poison" if you are a reviewer. I'm partial to > the more complex one adding an argument to @command. But it may not be > appropriate for stable. So a hacky fallback is provided as an alternative. I don't think either of them is good for stable. show is highly experimental extension, right? That said, the safest hack I can think of is adding null show command to the default command table, and wrap it by the show extension.
On Fri, Apr 28, 2017 at 10:04:48PM +0900, Yuya Nishihara wrote: > On Thu, 27 Apr 2017 16:11:34 -0700, Gregory Szorc wrote: > > This series is a "pick your poison" if you are a reviewer. I'm partial to > > the more complex one adding an argument to @command. But it may not be > > appropriate for stable. So a hacky fallback is provided as an alternative. > > I don't think either of them is good for stable. show is highly experimental > extension, right? > > That said, the safest hack I can think of is adding null show command to > the default command table, and wrap it by the show extension. I agree with Yuya here, even though it's a little frustrating to me. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Fri, Apr 28, 2017 at 6:04 AM, Yuya Nishihara <yuya@tcha.org> wrote: > On Thu, 27 Apr 2017 16:11:34 -0700, Gregory Szorc wrote: > > This series is a "pick your poison" if you are a reviewer. I'm partial to > > the more complex one adding an argument to @command. But it may not be > > appropriate for stable. So a hacky fallback is provided as an > alternative. > > I don't think either of them is good for stable. show is highly > experimental > extension, right? > > That said, the safest hack I can think of is adding null show command to > the default command table, and wrap it by the show extension. > Oh, I forgot to mention in the commit message that this was the first thing I did. It resulted in "show" appearing in help output, debugcomplete, etc. Unless I was doing it wrong, I thought this was worse than hacking alias resolution due to having user-visible side-effects.
On Fri, 28 Apr 2017 10:30:00 -0700, Gregory Szorc wrote: > On Fri, Apr 28, 2017 at 6:04 AM, Yuya Nishihara <yuya@tcha.org> wrote: > > On Thu, 27 Apr 2017 16:11:34 -0700, Gregory Szorc wrote: > > > This series is a "pick your poison" if you are a reviewer. I'm partial to > > > the more complex one adding an argument to @command. But it may not be > > > appropriate for stable. So a hacky fallback is provided as an > > alternative. > > > > I don't think either of them is good for stable. show is highly > > experimental > > extension, right? > > > > That said, the safest hack I can think of is adding null show command to > > the default command table, and wrap it by the show extension. > > > > Oh, I forgot to mention in the commit message that this was the first thing > I did. It resulted in "show" appearing in help output, debugcomplete, etc. > Unless I was doing it wrong, I thought this was worse than hacking alias > resolution due to having user-visible side-effects. Have you tried a command with no docstring (or "(EXPERIMENTAL)") ? I think the help issue can be addressed by that.
On Fri, Apr 28, 2017 at 4:04 PM, Yuya Nishihara <yuya@tcha.org> wrote: > On Fri, 28 Apr 2017 10:30:00 -0700, Gregory Szorc wrote: > > On Fri, Apr 28, 2017 at 6:04 AM, Yuya Nishihara <yuya@tcha.org> wrote: > > > On Thu, 27 Apr 2017 16:11:34 -0700, Gregory Szorc wrote: > > > > This series is a "pick your poison" if you are a reviewer. I'm > partial to > > > > the more complex one adding an argument to @command. But it may not > be > > > > appropriate for stable. So a hacky fallback is provided as an > > > alternative. > > > > > > I don't think either of them is good for stable. show is highly > > > experimental > > > extension, right? > > > > > > That said, the safest hack I can think of is adding null show command > to > > > the default command table, and wrap it by the show extension. > > > > > > > Oh, I forgot to mention in the commit message that this was the first > thing > > I did. It resulted in "show" appearing in help output, debugcomplete, > etc. > > Unless I was doing it wrong, I thought this was worse than hacking alias > > resolution due to having user-visible side-effects. > > Have you tried a command with no docstring (or "(EXPERIMENTAL)") ? I think > the help issue can be addressed by that. > No docstring displays "(no help text available)" in help. Docstring of "(EXPERIMENTAL)" renders an entry in hgweb's help. (This smells like a bug since help's CLI output doesn't change.) show also appears in debugcomplete. In both cases, we get a bunch of "extension 'show' overrides commands: show" warnings all over the place if we only do a simple @command in commands.py without hacking around the command table in show.py.
On Fri, 28 Apr 2017 19:24:42 -0700, Gregory Szorc wrote: > On Fri, Apr 28, 2017 at 4:04 PM, Yuya Nishihara <yuya@tcha.org> wrote: > > On Fri, 28 Apr 2017 10:30:00 -0700, Gregory Szorc wrote: > > > On Fri, Apr 28, 2017 at 6:04 AM, Yuya Nishihara <yuya@tcha.org> wrote: > > > > On Thu, 27 Apr 2017 16:11:34 -0700, Gregory Szorc wrote: > > > > > This series is a "pick your poison" if you are a reviewer. I'm > > partial to > > > > > the more complex one adding an argument to @command. But it may not > > be > > > > > appropriate for stable. So a hacky fallback is provided as an > > > > alternative. > > > > > > > > I don't think either of them is good for stable. show is highly > > > > experimental > > > > extension, right? > > > > > > > > That said, the safest hack I can think of is adding null show command > > to > > > > the default command table, and wrap it by the show extension. > > > > > > > > > > Oh, I forgot to mention in the commit message that this was the first > > thing > > > I did. It resulted in "show" appearing in help output, debugcomplete, > > etc. > > > Unless I was doing it wrong, I thought this was worse than hacking alias > > > resolution due to having user-visible side-effects. > > > > Have you tried a command with no docstring (or "(EXPERIMENTAL)") ? I think > > the help issue can be addressed by that. > > > > No docstring displays "(no help text available)" in help. Right. I got why I couldn't see it. My 'show' alias hid it. > Docstring of "(EXPERIMENTAL)" renders an entry in hgweb's help. (This > smells like a bug since help's CLI output doesn't change.) Yeah, it must be a bug. > show also appears in debugcomplete. > > In both cases, we get a bunch of "extension 'show' overrides commands: > show" warnings all over the place if we only do a simple @command in > commands.py without hacking around the command table in show.py. We'll have to use wrapcommand() instead. It seems keeping the show being extension causes troubles. We could add a hack to reserve the command name, but I don't think that's what the core should have to do. So, how about moving show to the core in 4.3? Still it can be marked as an EXPERIMENTAL command.
Patch
diff --git a/tests/test-show.t b/tests/test-show.t --- a/tests/test-show.t +++ b/tests/test-show.t @@ -1,3 +1,20 @@ +`hg show` without extension enabled says to enable the extension +TODO this is broken due to matching with showconfig + + $ hg show + defaults.backout=-d "0 0" + defaults.commit=-d "0 0" + defaults.shelve=--date "0 0" + defaults.tag=-d "0 0" + devel.all-warnings=true + largefiles.usercache=$TESTTMP/.cache/largefiles + ui.slash=True + ui.interactive=False + ui.mergemarkers=detailed + ui.promptecho=True + web.address=localhost + web.ipv6=False + $ cat >> $HGRCPATH << EOF > [extensions] > show =