Patchwork [7,of,7,v2] histedit: automatically select root with --commands

login
register
mail settings
Submitter timeless@mozdev.org
Date Dec. 28, 2015, 7:15 p.m.
Message ID <9d2a4cfe0bff5889f574.1451330103@waste.org>
Download mbox | patch
Permalink /patch/12377/
State Changes Requested
Headers show

Comments

timeless@mozdev.org - Dec. 28, 2015, 7:15 p.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1450856256 0
#      Wed Dec 23 07:37:36 2015 +0000
# Node ID 9d2a4cfe0bff5889f574cfbd804d60a2a7fddef1
# Parent  6eb6fa178ffafc7e1d5c2c89036877365fc6827e
histedit: automatically select root with --commands
Augie Fackler - Jan. 5, 2016, 4:51 p.m.
On Mon, Dec 28, 2015 at 01:15:03PM -0600, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1450856256 0
> #      Wed Dec 23 07:37:36 2015 +0000
> # Node ID 9d2a4cfe0bff5889f574cfbd804d60a2a7fddef1
> # Parent  6eb6fa178ffafc7e1d5c2c89036877365fc6827e
> histedit: automatically select root with --commands
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -803,7 +803,7 @@
>       ('f', 'force', False,
>        _('force outgoing even for unrelated repositories')),
>       ('r', 'rev', [], _('first revision to be edited'), _('REV'))],
> -     _("[OPTIONS] ([ANCESTOR] | --outgoing [URL])"))
> +     _("[OPTIONS] ([ANCESTOR] | --outgoing [URL] | --commands FILE)"))
>  def histedit(ui, repo, *freeargs, **opts):
>      """interactively edit changeset history
>
> @@ -830,6 +830,9 @@
>      - Use --outgoing -- it will be the first linear changeset not
>        included in destination. (See :hg:"help default-push")
>
> +    - Use --commands -- it will be the first linear changeset from the
> +      specified changesets.

I'm...not sure what this means. Can you reword the help?


> +
>      - Otherwise, the value from the "histedit.defaultrev" config option
>        is used as a revset to select the base revision when ANCESTOR is not
>        specified. The first revision returned by the revset is used. By
> @@ -929,6 +932,7 @@
>      force = opts.get('force')
>      rules = opts.get('commands', '')
>      revs = opts.get('rev', [])
> +    hasrevs = any(revs)
>      goal = 'new' # This invocation goal, in new, continue, abort
>      if force and not outg:
>          raise error.Abort(_('--force only allowed with --outgoing'))
> @@ -1074,6 +1078,19 @@
>              rules = f.read()
>              f.close()
>          actions = parserules(rules, state)
> +        if not any((outg, hasrevs)) and opts.get('commands', ''):
> +            try:
> +                avail = scmutil.revrange(repo,
> +                            [node.hex(a.node) for a in actions])
> +                rr = list(repo.set('roots(%ld)', avail))
> +                if len(rr) == 1:
> +                    root = rr[0].node()
> +                    revs = between(repo, root, topmost, state.keep)
> +                    ctxs = [repo[r] for r in revs]
> +            except error.RepoLookupError:
> +                pass
> +            except error.Abort:
> +                pass
>          verifyactions(actions, state, ctxs)
>
>          parentctxnode = repo[root].parents()[0].node()
> diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
> --- a/tests/test-histedit-arguments.t
> +++ b/tests/test-histedit-arguments.t
> @@ -100,6 +100,24 @@
>    > EOF
>    $ hg up --quiet
>
> +Test implicit minimum version via --commands
> +---------------------------------------
> +
> +  $ HGEDITOR=cat hg histedit --commands - << EOF
> +  > pick c8e68270e35a 3 four
> +  > pick 08d98a8350f3 4 five
> +  > EOF
> +  $ hg up --quiet
> +
> +  $ HGEDITOR=cat hg histedit --commands - << EOF
> +  > pick eb57da33312f 2 three
> +  > pick 08d98a8350f3 4 five
> +  > EOF
> +  abort: missing rules for changeset 579e40513370
> +  (use "drop 579e40513370" to discard, see also: "hg help -e histedit.config")
> +  [255]
> +  $ hg up --quiet
> +
>  Test config specified default
>  -----------------------------
>
> diff --git a/tests/test-histedit-commute.t b/tests/test-histedit-commute.t
> --- a/tests/test-histedit-commute.t
> +++ b/tests/test-histedit-commute.t
> @@ -268,7 +268,8 @@
>    > pick de71b079d9ce e
>    > pick 38b92f448761 c
>    > EOF
> -  abort: may not use "pick" with changesets other than the ones listed
> +  abort: pick "646537316230" changeset was not a candidate
> +  (only use listed changesets)
>    $ hg log --graph
>    @  changeset:   7:803ef1c6fcfd
>    |  tag:         tip
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
timeless - Jan. 5, 2016, 10:08 p.m.
> +    - Use --commands -- it will be the first linear changeset from the
> +      specified changesets.

Augie Fackler wrote:
> I'm...not sure what this means. Can you reword the help?

Imagine you have:
a->b->c->d->e->f

and you do --commands "pick f; roll d; pick c; mess e", it will use
`c` as the root.
If you did --commands "pick f; edit b; roll d; pick c; mess e", it
would use `b` as the root.

Having to manually say which the root is basically just annoying. It's
obvious as long as you aren't using automatic drops. (I really
wouldn't recommend this setting with automatic drops, although it
wouldn't be any more harmful tan not using it, in fact, it'd probably
be less harmful, since it'd only risk dropping things after you're
oldest changeset, instead of also things before it...)
Augie Fackler - Jan. 6, 2016, 9:21 p.m.
> On Jan 5, 2016, at 5:08 PM, timeless <timeless@gmail.com> wrote:
> 
>> +    - Use --commands -- it will be the first linear changeset from the
>> +      specified changesets.
> 
> Augie Fackler wrote:
>> I'm...not sure what this means. Can you reword the help?
> 
> Imagine you have:
> a->b->c->d->e->f
> 
> and you do --commands "pick f; roll d; pick c; mess e", it will use
> `c` as the root.
> If you did --commands "pick f; edit b; roll d; pick c; mess e", it
> would use `b` as the root.
> 
> Having to manually say which the root is basically just annoying. It's
> obvious as long as you aren't using automatic drops. (I really
> wouldn't recommend this setting with automatic drops, although it
> wouldn't be any more harmful tan not using it, in fact, it'd probably
> be less harmful, since it'd only risk dropping things after you're
> oldest changeset, instead of also things before it...)

I mentioned this in IRC, but this is just too magical to me. Can anyone else write a convincing case for it?

(It’s partially too magical because of the implicit drop option, because if that and this are used together you’re well into footgun territory, which I desperately want to avoid.)

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -803,7 +803,7 @@ 
      ('f', 'force', False,
       _('force outgoing even for unrelated repositories')),
      ('r', 'rev', [], _('first revision to be edited'), _('REV'))],
-     _("[OPTIONS] ([ANCESTOR] | --outgoing [URL])"))
+     _("[OPTIONS] ([ANCESTOR] | --outgoing [URL] | --commands FILE)"))
 def histedit(ui, repo, *freeargs, **opts):
     """interactively edit changeset history
 
@@ -830,6 +830,9 @@ 
     - Use --outgoing -- it will be the first linear changeset not
       included in destination. (See :hg:"help default-push")
 
+    - Use --commands -- it will be the first linear changeset from the
+      specified changesets.
+
     - Otherwise, the value from the "histedit.defaultrev" config option
       is used as a revset to select the base revision when ANCESTOR is not
       specified. The first revision returned by the revset is used. By
@@ -929,6 +932,7 @@ 
     force = opts.get('force')
     rules = opts.get('commands', '')
     revs = opts.get('rev', [])
+    hasrevs = any(revs)
     goal = 'new' # This invocation goal, in new, continue, abort
     if force and not outg:
         raise error.Abort(_('--force only allowed with --outgoing'))
@@ -1074,6 +1078,19 @@ 
             rules = f.read()
             f.close()
         actions = parserules(rules, state)
+        if not any((outg, hasrevs)) and opts.get('commands', ''):
+            try:
+                avail = scmutil.revrange(repo,
+                            [node.hex(a.node) for a in actions])
+                rr = list(repo.set('roots(%ld)', avail))
+                if len(rr) == 1:
+                    root = rr[0].node()
+                    revs = between(repo, root, topmost, state.keep)
+                    ctxs = [repo[r] for r in revs]
+            except error.RepoLookupError:
+                pass
+            except error.Abort:
+                pass
         verifyactions(actions, state, ctxs)
 
         parentctxnode = repo[root].parents()[0].node()
diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
--- a/tests/test-histedit-arguments.t
+++ b/tests/test-histedit-arguments.t
@@ -100,6 +100,24 @@ 
   > EOF
   $ hg up --quiet
 
+Test implicit minimum version via --commands
+---------------------------------------
+
+  $ HGEDITOR=cat hg histedit --commands - << EOF
+  > pick c8e68270e35a 3 four
+  > pick 08d98a8350f3 4 five
+  > EOF
+  $ hg up --quiet
+
+  $ HGEDITOR=cat hg histedit --commands - << EOF
+  > pick eb57da33312f 2 three
+  > pick 08d98a8350f3 4 five
+  > EOF
+  abort: missing rules for changeset 579e40513370
+  (use "drop 579e40513370" to discard, see also: "hg help -e histedit.config")
+  [255]
+  $ hg up --quiet
+
 Test config specified default
 -----------------------------
 
diff --git a/tests/test-histedit-commute.t b/tests/test-histedit-commute.t
--- a/tests/test-histedit-commute.t
+++ b/tests/test-histedit-commute.t
@@ -268,7 +268,8 @@ 
   > pick de71b079d9ce e
   > pick 38b92f448761 c
   > EOF
-  abort: may not use "pick" with changesets other than the ones listed
+  abort: pick "646537316230" changeset was not a candidate
+  (only use listed changesets)
   $ hg log --graph
   @  changeset:   7:803ef1c6fcfd
   |  tag:         tip