Patchwork [03,of,11,RFC] histedit: add rule handling for exec

login
register
mail settings
Submitter Olle Lundberg
Date March 5, 2014, 4:15 p.m.
Message ID <125d473d0d3982c8a01c.1394036114@SE-C02KQ0DADR55>
Download mbox | patch
Permalink /patch/3854/
State Changes Requested
Headers show

Comments

Olle Lundberg - March 5, 2014, 4:15 p.m.
# HG changeset patch
# User Olle Lundberg <geek@nerd.sh>
# Date 1394034350 -3600
#      Wed Mar 05 16:45:50 2014 +0100
# Node ID 125d473d0d3982c8a01c94acbcec350522d9555e
# Parent  52e135b90ce2b4719d7f550133a01d895a29c2bc
histedit: add rule handling for exec

This changeset modifies verifyrules to skip the repo and hash
related logic since we are executing a command that is agnostic
to the repo state and context.
David Soria Parra - March 5, 2014, 9:38 p.m.
Olle Lundberg <olle.lundberg@gmail.com> writes:

> # HG changeset patch
> # User Olle Lundberg <geek@nerd.sh>
> # Date 1394034350 -3600
> #      Wed Mar 05 16:45:50 2014 +0100
> # Node ID 125d473d0d3982c8a01c94acbcec350522d9555e
> # Parent  52e135b90ce2b4719d7f550133a01d895a29c2bc
> histedit: add rule handling for exec
>
> This changeset modifies verifyrules to skip the repo and hash
> related logic since we are executing a command that is agnostic
> to the repo state and context.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -769,24 +769,28 @@
>      seen = set()
>      for r in rules:
>          if ' ' not in r:
>              raise util.Abort(_('malformed line "%s"') % r)
>          action, rest = r.split(' ', 1)
> -        ha = rest.strip().split(' ', 1)[0]
> -        try:
> -            ha = str(repo[ha])  # ensure its a short hash
> -        except error.RepoError:
> -            raise util.Abort(_('unknown changeset %s listed') % ha)
> -        if ha not in expected:
> -            raise util.Abort(
> -                _('may not use changesets other than the ones listed'))
> -        if ha in seen:
> -            raise util.Abort(_('duplicated command for changeset %s') % ha)
> -        seen.add(ha)
> +        if action not in ('x', 'exec'):
> +            args = rest.strip().split(' ', 1)[0]
> +            try:

I don't think his option is necessary or needed at all. Histedit doesn't
have any commands to modify the commandslist and I think it shouldn't
be part of histedit to modify the commandset itself. I highly recommend
dropping --exec completly.
Olle Lundberg - March 5, 2014, 9:54 p.m.
On Wed, Mar 5, 2014 at 10:38 PM, David Soria Parra <
dsp@experimentalworks.net> wrote:

> Olle Lundberg <olle.lundberg@gmail.com> writes:
>
> > # HG changeset patch
> > # User Olle Lundberg <geek@nerd.sh>
> > # Date 1394034350 -3600
> > #      Wed Mar 05 16:45:50 2014 +0100
> > # Node ID 125d473d0d3982c8a01c94acbcec350522d9555e
> > # Parent  52e135b90ce2b4719d7f550133a01d895a29c2bc
> > histedit: add rule handling for exec
> >
> > This changeset modifies verifyrules to skip the repo and hash
> > related logic since we are executing a command that is agnostic
> > to the repo state and context.
> >
> > diff --git a/hgext/histedit.py b/hgext/histedit.py
> > --- a/hgext/histedit.py
> > +++ b/hgext/histedit.py
> > @@ -769,24 +769,28 @@
> >      seen = set()
> >      for r in rules:
> >          if ' ' not in r:
> >              raise util.Abort(_('malformed line "%s"') % r)
> >          action, rest = r.split(' ', 1)
> > -        ha = rest.strip().split(' ', 1)[0]
> > -        try:
> > -            ha = str(repo[ha])  # ensure its a short hash
> > -        except error.RepoError:
> > -            raise util.Abort(_('unknown changeset %s listed') % ha)
> > -        if ha not in expected:
> > -            raise util.Abort(
> > -                _('may not use changesets other than the ones listed'))
> > -        if ha in seen:
> > -            raise util.Abort(_('duplicated command for changeset %s') %
> ha)
> > -        seen.add(ha)
> > +        if action not in ('x', 'exec'):
> > +            args = rest.strip().split(' ', 1)[0]
> > +            try:
>
> I don't think his option is necessary or needed at all. Histedit doesn't
> have any commands to modify the commandslist and I think it shouldn't
> be part of histedit to modify the commandset itself. I highly recommend
> dropping --exec completly.
>

I think you are commenting on the wrong patch. This is an internal
implementation detail. The user facing --exec was in patch 10

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -769,24 +769,28 @@ 
     seen = set()
     for r in rules:
         if ' ' not in r:
             raise util.Abort(_('malformed line "%s"') % r)
         action, rest = r.split(' ', 1)
-        ha = rest.strip().split(' ', 1)[0]
-        try:
-            ha = str(repo[ha])  # ensure its a short hash
-        except error.RepoError:
-            raise util.Abort(_('unknown changeset %s listed') % ha)
-        if ha not in expected:
-            raise util.Abort(
-                _('may not use changesets other than the ones listed'))
-        if ha in seen:
-            raise util.Abort(_('duplicated command for changeset %s') % ha)
-        seen.add(ha)
+        if action not in ('x', 'exec'):
+            args = rest.strip().split(' ', 1)[0]
+            try:
+                args = str(repo[args])  # ensure its a short hash
+            except error.RepoError:
+                raise util.Abort(_('unknown changeset %s listed') % args)
+            if args not in expected:
+                raise util.Abort(
+                    _('may not use changesets other than the ones listed'))
+            if args in seen:
+                raise util.Abort(
+                    _('duplicated command for changeset %s') % args)
+            seen.add(args)
+        else:
+            args = rest
         if action not in actiontable:
             raise util.Abort(_('unknown action "%s"') % action)
-        parsed.append([action, ha])
+        parsed.append([action, args])
     missing = sorted(expected - seen)  # sort to stabilize output
     if missing:
         raise util.Abort(_('missing rules for changeset %s') % missing[0],
                          hint=_('do you want to use the drop action?'))
     return parsed