( )⚙ D8714 state: support validated declaration of nested unfinished ops

This is an archive of the discontinued Mercurial Phabricator instance.

state: support validated declaration of nested unfinished ops
ClosedPublic

Authored by dploch on Jul 8 2020, 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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

dploch created this revision.Jul 8 2020, 6:50 PM
mjacob added a subscriber: mjacob.Jul 8 2020, 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.Jul 14 2020, 4:38 PM
pulkit added a subscriber: pulkit.Jul 17 2020, 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.Jul 17 2020, 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.Jul 20 2020, 8:13 AM

This looks quite interesting.

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