Page MenuHomePhabricator

state: support validated declaration of nested unfinished ops
ClosedPublic

Authored by dploch on Wed, Jul 8, 6:50 PM.

Details

Summary

This enables extensions to define commands that delgate to rebase, evolve, etc. one or more times to also have their own unfinished states for the full sequence of operations without monkey-patching _unfinishedstates.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dploch created this revision.Wed, Jul 8, 6:50 PM
mjacob added a subscriber: mjacob.Wed, Jul 8, 8:28 PM

Can you give an example of an extension that would benefit from this? It’s not immediately obvious to me, and ideally patches should be written such that it’s obvious for most people. :)

Can you add a test case? That would probably also help understanding the use case.

Can you give an example of an extension that would benefit from this? It’s not immediately obvious to me, and ideally patches should be written such that it’s obvious for most people. :)

To give a concrete example, we have an internal command which does something similar to chainify. It generates N individual commits with different parents (based on input flags), then rebases then sequentially into a single chain in a specific order; it currently does some hacky wrapping of methods in this file to make it work. We have need of another command that does something similar with multiple evolve instructions; rather than repeat the hacks, I thought it would be good to make the support cleaner by implementing it upstream.

Can you add a test case? That would probably also help understanding the use case.

I added a test file with a simple-ish example extension that hopefully clarifies the intent. I revised the changes a bit to make the intended usage in extensions cleaner/more intuitive (I probably should have done this in the first place since there were also two minor bugs in the initial draft.)

dploch updated this revision to Diff 21904.Tue, Jul 14, 4:38 PM
pulkit added a subscriber: pulkit.Fri, Jul 17, 1:13 PM

Following test fails with this patch:

--- /tests/test-check-code.t
+++ /tests/test-check-code.t.err
@@ -33,6 +33,16 @@
   Skipping i18n/polib.py it has no-che?k-code (glob)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
   Skipping tests/badserverext.py it has no-che?k-code (glob)
+  tests/test-state-extension.t:79:
+   >   $ hg co -q .^
+   warning: ^ must be quoted
+  tests/test-state-extension.t:82:
+   >   $ hg co -q .^
+   warning: ^ must be quoted
+  tests/test-state-extension.t:85:
+   >   $ hg co -q .^
+   warning: ^ must be quoted
+  [1]
 
 @commands in debugcommands.py should be in alphabetical order.
 

ERROR: test-check-code.t output changed
!
--- /tests/test-check-pylint.t
+++ /tests/test-check-pylint.t.err
@@ -20,3 +20,9 @@
   ------------------------------------ (?)
   Your code has been rated at 10.00/10 (?)
    (?)
+  ************* Module mercurial.state
+  W:200, 0: Dangerous default value [] as argument (dangerous-default-value)
+  
+  ------------------------------------
+  Your code has been rated at 10.00/10
+  

ERROR: test-check-pylint.t output changed
!
dploch updated this revision to Diff 21962.Fri, Jul 17, 8:04 PM

Following test fails with this patch:

--- /tests/test-check-code.t
+++ /tests/test-check-code.t.err
@@ -33,6 +33,16 @@
   Skipping i18n/polib.py it has no-che?k-code (glob)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
   Skipping tests/badserverext.py it has no-che?k-code (glob)
+  tests/test-state-extension.t:79:
+   >   $ hg co -q .^
+   warning: ^ must be quoted
+  tests/test-state-extension.t:82:
+   >   $ hg co -q .^
+   warning: ^ must be quoted
+  tests/test-state-extension.t:85:
+   >   $ hg co -q .^
+   warning: ^ must be quoted
+  [1]
 @commands in debugcommands.py should be in alphabetical order.
ERROR: test-check-code.t output changed
!
--- /tests/test-check-pylint.t
+++ /tests/test-check-pylint.t.err
@@ -20,3 +20,9 @@
   ------------------------------------ (?)
   Your code has been rated at 10.00/10 (?)
    (?)
+  ************* Module mercurial.state
+  W:200, 0: Dangerous default value [] as argument (dangerous-default-value)
+  
+  ------------------------------------
+  Your code has been rated at 10.00/10
+  
ERROR: test-check-pylint.t output changed
!

Fixed.

pulkit accepted this revision.Mon, Jul 20, 8:13 AM

This looks quite interesting.

This revision is now accepted and ready to land.Mon, Jul 20, 8:13 AM