Patchwork [1,of,2,STABLE] tests: demonstrate `hg show` mapping to showconfig

login
register
mail settings
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

Gregory Szorc - April 27, 2017, 11:08 p.m.
# 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.
Gregory Szorc - April 27, 2017, 11:11 p.m.
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 =
>
Yuya Nishihara - April 28, 2017, 1:04 p.m.
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.
Augie Fackler - April 28, 2017, 2:16 p.m.
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
Gregory Szorc - April 28, 2017, 5:30 p.m.
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.
Yuya Nishihara - April 28, 2017, 11:04 p.m.
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.
Gregory Szorc - April 29, 2017, 2:24 a.m.
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.
Yuya Nishihara - April 29, 2017, 2:16 p.m.
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 =