Re: [PATCH 2 of 2 RFC] dispatch: look up command by [:] syntax (PoC)

[Yuya Nishihara]

On Sat, 24 Feb 2018 18:03:49 -0800, Gregory Szorc wrote:
> On Sat, Feb 24, 2018 at 5:42 PM, Yuya Nishihara  wrote:
> > On Sat, 24 Feb 2018 11:39:26 -0800, Gregory Szorc wrote:
> > > I will say that I believe we had general consensus at a previous sprint
> > to
> > > implement proper support for sub-commands. This would allow us to
> > implement
> > > arguments on individual show views. e.g. `hg show work --maxage 14d`. It
> > > could also be useful for commands like bookmarks and topics, which try to
> > > shoehorn lots of functionality into a single command. From a UX
> > > perspective, I think explicit verbs are better than dash prefixed
> > arguments
> > > for controlling mode of operation. e.g. `hg bookmark delete foo`
> > (possibly
> > > bad example due to BC concerns).
> > >
> > > If this is the direction we want to go in, I'd be hesitant to implement
> > > [alias] support unless we're sure it would be compatible with proper
> > > sub-commands.
> >
> > Ah, okay, I didn't know the sub-command thingy. Let's revisit after that.
> > I don't wanna make the dispatcher more complicated.
> >
> 
> Well, nobody is actively working on sub-commands. So if you see a way to
> make this aliasing thing work such that it is forward compatible with
> sub-commands, then my vote is to do it.

I have no idea how sub-commands will be handled internally. I think sub-commands
and alias-bypass are orthogonal feature, but they may have some conflicts at
syntax level.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 RFC] dispatch: look up command by [:] syntax (PoC)

[Gregory Szorc]

On Sat, Feb 24, 2018 at 5:42 PM, Yuya Nishihara  wrote:

> On Sat, 24 Feb 2018 11:39:26 -0800, Gregory Szorc wrote:
> > I will say that I believe we had general consensus at a previous sprint
> to
> > implement proper support for sub-commands. This would allow us to
> implement
> > arguments on individual show views. e.g. `hg show work --maxage 14d`. It
> > could also be useful for commands like bookmarks and topics, which try to
> > shoehorn lots of functionality into a single command. From a UX
> > perspective, I think explicit verbs are better than dash prefixed
> arguments
> > for controlling mode of operation. e.g. `hg bookmark delete foo`
> (possibly
> > bad example due to BC concerns).
> >
> > If this is the direction we want to go in, I'd be hesitant to implement
> > [alias] support unless we're sure it would be compatible with proper
> > sub-commands.
>
> Ah, okay, I didn't know the sub-command thingy. Let's revisit after that.
> I don't wanna make the dispatcher more complicated.
>

Well, nobody is actively working on sub-commands. So if you see a way to
make this aliasing thing work such that it is forward compatible with
sub-commands, then my vote is to do it.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 RFC] dispatch: look up command by [:] syntax (PoC)

[Yuya Nishihara]

On Sat, 24 Feb 2018 11:39:26 -0800, Gregory Szorc wrote:
> I will say that I believe we had general consensus at a previous sprint to
> implement proper support for sub-commands. This would allow us to implement
> arguments on individual show views. e.g. `hg show work --maxage 14d`. It
> could also be useful for commands like bookmarks and topics, which try to
> shoehorn lots of functionality into a single command. From a UX
> perspective, I think explicit verbs are better than dash prefixed arguments
> for controlling mode of operation. e.g. `hg bookmark delete foo` (possibly
> bad example due to BC concerns).
> 
> If this is the direction we want to go in, I'd be hesitant to implement
> [alias] support unless we're sure it would be compatible with proper
> sub-commands.

Ah, okay, I didn't know the sub-command thingy. Let's revisit after that.
I don't wanna make the dispatcher more complicated.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 RFC] dispatch: look up command by [:] syntax (PoC)

[Gregory Szorc]

On Thu, Feb 22, 2018 at 6:54 AM, Yuya Nishihara  wrote:

> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1516798904 -32400
> #  Wed Jan 24 22:01:44 2018 +0900
> # Node ID cb9e1cad42a9a13f17c4c75d350cd509b08f4a21
> # Parent  e2030eaec92b1ed12577cbe48cd0495d106818a9
> dispatch: look up command by [:] syntax (PoC)
>
> This allows us to run the show command without giving up our show alias.
> The separator ':' is copied from the merge-tools syntax,
> ":".
>
>   [alias]
>   show = log -pvr
>   work = show:show work
>
>   $ hg work
>
> So, do we like it?
>

I don't know the code that well, so I can't comment on the patch.

I will say that I believe we had general consensus at a previous sprint to
implement proper support for sub-commands. This would allow us to implement
arguments on individual show views. e.g. `hg show work --maxage 14d`. It
could also be useful for commands like bookmarks and topics, which try to
shoehorn lots of functionality into a single command. From a UX
perspective, I think explicit verbs are better than dash prefixed arguments
for controlling mode of operation. e.g. `hg bookmark delete foo` (possibly
bad example due to BC concerns).

If this is the direction we want to go in, I'd be hesitant to implement
[alias] support unless we're sure it would be compatible with proper
sub-commands.


>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -707,6 +707,19 @@ def findcmd(cmd, table, strict=True):
>
>  raise error.UnknownCommand(cmd, allcmds)
>
> +def findcmdspace(name, commands, strict=True):
> +"""Look up (aliases, command table entry) from commands module"""
> +# TODO: ':' vs '.'
> +ns, sep, cmd = name.partition(':')
> +if not sep:
> +return findcmd(name, commands.table, strict=strict)
> +table = commands.namespace.get(ns)
> +if table is None:
> +raise error.UnknownCommand(name)
> +# TODO: wrap Ambiguous/UnknownCommand exception to return a full name?
> +# TODO: might want to require strict=True for namespaced name
> +return findcmd(cmd, table, strict=strict)
> +
>  def changebranch(ui, repo, revs, label):
>  """ Change the branch name of given revs to label """
>
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -234,7 +234,7 @@ def _runcatch(req):
>  try:
>  cmdargs = fancyopts.fancyopts(req.args[:],
> commands.globalopts, {})
>  cmd = cmdargs[0]
> -aliases, entry = cmdutil.findcmd(cmd, commands.table, False)
> +aliases, entry = cmdutil.findcmdspace(cmd, commands,
> strict=False)
>  realcmd = aliases[0]
>  except (error.UnknownCommand, error.AmbiguousCommand,
>  IndexError, getopt.GetoptError):
> @@ -602,8 +602,8 @@ def _parse(ui, args):
>
>  if args:
>  cmd, args = args[0], args[1:]
> -aliases, entry = cmdutil.findcmd(cmd, commands.table,
> - ui.configbool("ui", "strict"))
> +aliases, entry = cmdutil.findcmdspace(cmd, commands,
> +  ui.configbool("ui",
> "strict"))
>  cmd = aliases[0]
>  args = aliasargs(entry[0], args)
>  defaults = ui.config("defaults", cmd)
> diff --git a/mercurial/help.py b/mercurial/help.py
> --- a/mercurial/help.py
> +++ b/mercurial/help.py
> @@ -322,8 +322,8 @@ def help_(ui, commands, name, unknowncmd
>
>  def helpcmd(name, subtopic=None):
>  try:
> -aliases, entry = cmdutil.findcmd(name, commands.table,
> - strict=unknowncmd)
> +aliases, entry = cmdutil.findcmdspace(name, commands,
> +  strict=unknowncmd)
>  except error.AmbiguousCommand as inst:
>  # py3k fix: except vars can't be used outside the scope of the
>  # except block, nor can be used inside a lambda. python
> issue4617
> @@ -524,7 +524,7 @@ def help_(ui, commands, name, unknowncmd
>  indicateomitted(rst, omitted)
>
>  try:
> -cmdutil.findcmd(name, commands.table)
> +cmdutil.findcmdspace(name, commands)
>  rst.append(_("\nuse 'hg help -c %s' to see help for "
> "the %s command\n") % (name, name))
>  except error.UnknownCommand:
> diff --git a/tests/test-dispatch.t b/tests/test-dispatch.t
> --- a/tests/test-dispatch.t
> +++ b/tests/test-dispatch.t
> @@ -11,6 +11,24 @@ Redundant options used to crash (issue43
>$ hg ci -Ama
>adding a
>
> +Look up command by canonical name:
> +
> +  $ hg :log
> +  changeset:   0:cb9a9f314b8b
> +  tag: tip
> +  user:test
> +  date:Thu Jan 01 00:00:00 1970 +
> +  summary: a
> +
> +  $ hg --config alias.log='log -G' 

Re: [PATCH 2 of 2 RFC] dispatch: look up command by [:] syntax (PoC)

[Yuya Nishihara]

On Fri, 23 Feb 2018 13:25:52 +0800, Anton Shestakov wrote:
> On Thu, 22 Feb 2018 23:54:47 +0900
> Yuya Nishihara  wrote:
> 
> > # HG changeset patch
> > # User Yuya Nishihara 
> > # Date 1516798904 -32400
> > #  Wed Jan 24 22:01:44 2018 +0900
> > # Node ID cb9e1cad42a9a13f17c4c75d350cd509b08f4a21
> > # Parent  e2030eaec92b1ed12577cbe48cd0495d106818a9
> > dispatch: look up command by [:] syntax (PoC)
> > 
> > This allows us to run the show command without giving up our show alias.
> > The separator ':' is copied from the merge-tools syntax, ":".
> > 
> >   [alias]
> >   show = log -pvr
> >   work = show:show work
> 
> This syntax looks alright to me as a user, but maybe this should also
> support "internal:foo" form in addition to ":foo", in case people want
> to spell it out, and to be fully compatible syntax-wise with
> merge-tools.

That would conflict with the "internal" extension if we had. That's unlikely
in practice, but I don't want to special-case it.

> My thought is that if somebody asks why not use backslash
> with core commands to avoid aliases (like in bash, e.g. \ls) then we
> can say that this is simply 100% reuse of pre-existing syntax (in
> addition to mentioning namespaces for extensions).

Hmm, nobody would want double backslashes, I suppose.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 RFC] dispatch: look up command by [:] syntax (PoC)

[Anton Shestakov]

On Thu, 22 Feb 2018 23:54:47 +0900
Yuya Nishihara  wrote:

> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1516798904 -32400
> #  Wed Jan 24 22:01:44 2018 +0900
> # Node ID cb9e1cad42a9a13f17c4c75d350cd509b08f4a21
> # Parent  e2030eaec92b1ed12577cbe48cd0495d106818a9
> dispatch: look up command by [:] syntax (PoC)
> 
> This allows us to run the show command without giving up our show alias.
> The separator ':' is copied from the merge-tools syntax, ":".
> 
>   [alias]
>   show = log -pvr
>   work = show:show work

This syntax looks alright to me as a user, but maybe this should also
support "internal:foo" form in addition to ":foo", in case people want
to spell it out, and to be fully compatible syntax-wise with
merge-tools. My thought is that if somebody asks why not use backslash
with core commands to avoid aliases (like in bash, e.g. \ls) then we
can say that this is simply 100% reuse of pre-existing syntax (in
addition to mentioning namespaces for extensions).
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 2 RFC] dispatch: look up command by [:] syntax (PoC)

[Yuya Nishihara]

# HG changeset patch
# User Yuya Nishihara 
# Date 1516798904 -32400
#  Wed Jan 24 22:01:44 2018 +0900
# Node ID cb9e1cad42a9a13f17c4c75d350cd509b08f4a21
# Parent  e2030eaec92b1ed12577cbe48cd0495d106818a9
dispatch: look up command by [:] syntax (PoC)

This allows us to run the show command without giving up our show alias.
The separator ':' is copied from the merge-tools syntax, ":".

  [alias]
  show = log -pvr
  work = show:show work

  $ hg work

So, do we like it?

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -707,6 +707,19 @@ def findcmd(cmd, table, strict=True):
 
 raise error.UnknownCommand(cmd, allcmds)
 
+def findcmdspace(name, commands, strict=True):
+"""Look up (aliases, command table entry) from commands module"""
+# TODO: ':' vs '.'
+ns, sep, cmd = name.partition(':')
+if not sep:
+return findcmd(name, commands.table, strict=strict)
+table = commands.namespace.get(ns)
+if table is None:
+raise error.UnknownCommand(name)
+# TODO: wrap Ambiguous/UnknownCommand exception to return a full name?
+# TODO: might want to require strict=True for namespaced name
+return findcmd(cmd, table, strict=strict)
+
 def changebranch(ui, repo, revs, label):
 """ Change the branch name of given revs to label """
 
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -234,7 +234,7 @@ def _runcatch(req):
 try:
 cmdargs = fancyopts.fancyopts(req.args[:], commands.globalopts, {})
 cmd = cmdargs[0]
-aliases, entry = cmdutil.findcmd(cmd, commands.table, False)
+aliases, entry = cmdutil.findcmdspace(cmd, commands, strict=False)
 realcmd = aliases[0]
 except (error.UnknownCommand, error.AmbiguousCommand,
 IndexError, getopt.GetoptError):
@@ -602,8 +602,8 @@ def _parse(ui, args):
 
 if args:
 cmd, args = args[0], args[1:]
-aliases, entry = cmdutil.findcmd(cmd, commands.table,
- ui.configbool("ui", "strict"))
+aliases, entry = cmdutil.findcmdspace(cmd, commands,
+  ui.configbool("ui", "strict"))
 cmd = aliases[0]
 args = aliasargs(entry[0], args)
 defaults = ui.config("defaults", cmd)
diff --git a/mercurial/help.py b/mercurial/help.py
--- a/mercurial/help.py
+++ b/mercurial/help.py
@@ -322,8 +322,8 @@ def help_(ui, commands, name, unknowncmd
 
 def helpcmd(name, subtopic=None):
 try:
-aliases, entry = cmdutil.findcmd(name, commands.table,
- strict=unknowncmd)
+aliases, entry = cmdutil.findcmdspace(name, commands,
+  strict=unknowncmd)
 except error.AmbiguousCommand as inst:
 # py3k fix: except vars can't be used outside the scope of the
 # except block, nor can be used inside a lambda. python issue4617
@@ -524,7 +524,7 @@ def help_(ui, commands, name, unknowncmd
 indicateomitted(rst, omitted)
 
 try:
-cmdutil.findcmd(name, commands.table)
+cmdutil.findcmdspace(name, commands)
 rst.append(_("\nuse 'hg help -c %s' to see help for "
"the %s command\n") % (name, name))
 except error.UnknownCommand:
diff --git a/tests/test-dispatch.t b/tests/test-dispatch.t
--- a/tests/test-dispatch.t
+++ b/tests/test-dispatch.t
@@ -11,6 +11,24 @@ Redundant options used to crash (issue43
   $ hg ci -Ama
   adding a
 
+Look up command by canonical name:
+
+  $ hg :log
+  changeset:   0:cb9a9f314b8b
+  tag: tip
+  user:test
+  date:Thu Jan 01 00:00:00 1970 +
+  summary: a
+  
+  $ hg --config alias.log='log -G' :log
+  changeset:   0:cb9a9f314b8b
+  tag: tip
+  user:test
+  date:Thu Jan 01 00:00:00 1970 +
+  summary: a
+  
+  $ hg --config extensions.purge= purge:purge
+
 Missing arg:
 
   $ hg cat
diff --git a/tests/test-help.t b/tests/test-help.t
--- a/tests/test-help.t
+++ b/tests/test-help.t
@@ -496,6 +496,14 @@ Test ambiguous command help
   
   (use 'hg help -v ad' to show built-in aliases and global options)
 
+  $ hg help :ad
+  list of commands:
+  
+   add   add the specified files on the next commit
+   addremove add all new files, delete all missing files
+  
+  (use 'hg help -v :ad' to show built-in aliases and global options)
+
 Test command without options
 
   $ hg help verify
@@ -670,6 +678,13 @@ Test command without options
   (use 'hg help' for the full list of commands or 'hg -v' for details)
   [255]
 
+Look up command by canonical name:
+
+  $ hg --config alias.status=log help status | grep 'alias for'
+  alias for: hg log
+  $ hg --config alias.status=log help :status |