( )⚙ D286 undo: no argument redo

This is an archive of the discontinued Mercurial Phabricator instance.

undo: no argument redo
ClosedPublic

Authored by felixmerk on Aug 8 2017, 6:15 PM.
Tags
None
Subscribers

Details

Reviewers
quark
Group Reviewers
Restricted Project
Commits
rFBHGX916de186acdc: undo: no argument redo
Summary

Changes redo to straight up undo the previous undo. This is a bit trickier than
it may seem. The previous redo capabilities are still covered in hg undo as
--index accepts negative numbers allowing you to step both forward and
backwards one step at a time. The new redo reads the linear undo log to find
out where to undo to while updating the undo/redo index allowing you to continue
from where you left off.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

felixmerk created this revision.Aug 8 2017, 6:15 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 8 2017, 6:15 PM
stash added a subscriber: stash.Aug 9 2017, 11:10 AM

Sorry, only nits so far. Will look more closely later

hgext3rd/undo.py
584

Second and reverseindex != -1 is not necessary

671–689

() are not necessary

696

Same here

felixmerk added inline comments.Aug 9 2017, 1:50 PM
hgext3rd/undo.py
584

I want to allow hg undo --index -1 --branch #. Not only is this somewhat useful for the user, it allows for interactive branch undoes if I ever want to add that. (--index -1 is stepping one repo state forward)

felixmerk updated this revision to Diff 710.Aug 9 2017, 2:10 PM
quark requested changes to this revision.EditedAug 9 2017, 6:46 PM
quark added a subscriber: quark.

The test looks solid. Great!

Parsing the undo command line flags manually could easily miss corner cases. I'd suggest use what Mercurial actually uses:

opts = {}
mercurial.fancyopts.fancyopts(['--ind=4', '-fa'], cmdtable['undo'][1], opts)
opts # {'absolute': True, 'branch': '', 'force': True, 'index': 4, 'keep': False}
hgext3rd/undo.py
676–685

--index=4 is also a valid way to provide the flag.

691–694

Could be simplify to if i in ('-a', '--absolute'):

This revision now requires changes to proceed.Aug 9 2017, 6:46 PM
felixmerk edited edge metadata.Aug 10 2017, 1:55 PM
felixmerk updated this revision to Diff 741.
felixmerk updated this revision to Diff 814.Aug 11 2017, 5:39 PM
quark accepted this revision.Aug 11 2017, 8:57 PM
This revision is now accepted and ready to land.Aug 11 2017, 8:57 PM
This revision was automatically updated to reflect the committed changes.