Created on 2014-07-23.19:23:31 by mdiaz, last changed 2018-06-24.09:46:13 by ganesh.
File name |
Status |
Uploaded |
Type |
Edit |
Remove |
implementation-of-the-command-_darcs-undo__.dpatch
|
|
mdiaz,
2014-07-23.19:23:31
|
application/x-darcs-patch |
|
|
implementation-of-the-command-_darcs-undo__.dpatch
|
|
mdiaz,
2014-07-29.13:33:18
|
application/x-darcs-patch |
|
|
implementation-of-the-command-_darcs-undo__.dpatch
|
|
mdiaz,
2014-07-30.23:50:20
|
application/x-darcs-patch |
|
|
implementation-of-the-commands-_undo_-and-_redo__.dpatch
|
|
mdiaz,
2014-08-18.12:07:39
|
application/x-darcs-patch |
|
|
patch-preview.txt
|
|
mdiaz,
2014-07-23.19:23:31
|
text/x-darcs-patch |
|
|
patch-preview.txt
|
|
mdiaz,
2014-07-29.13:33:18
|
text/x-darcs-patch |
|
|
patch-preview.txt
|
|
mdiaz,
2014-07-30.23:50:20
|
text/x-darcs-patch |
|
|
patch-preview.txt
|
|
mdiaz,
2014-08-18.12:07:39
|
text/x-darcs-patch |
|
|
unnamed
|
|
mdiaz,
2014-07-23.19:23:31
|
|
|
|
unnamed
|
|
mdiaz,
2014-07-29.13:33:18
|
|
|
|
unnamed
|
|
mdiaz,
2014-07-30.23:50:20
|
|
|
|
unnamed
|
|
mdiaz,
2014-08-18.12:07:39
|
|
|
|
See mailing list archives
for discussion on individual patches.
msg17627 (view) |
Author: mdiaz |
Date: 2014-07-23.19:23:31 |
|
First implementation of the command 'darcs undo'.
This implementation only makes a copy (named 'previous_hashed_inventory')
of the 'hashed_inventory' file using the function finalizeRepositoryChanges
in the module Repository.Internal.
The command 'darcs undo' swap the content of the files 'hashed_inventory'
and 'previous_hashed_inventory'.
Using this command is possible for example, to undo the last run of
the command amend-record.
1 patch for repository http://darcs.net/screened:
Wed Jul 23 16:09:16 ART 2014 Marcio Diaz <marcio.diaz@gmail.com>
* Implementation of the command 'darcs undo'.
Attachments
|
msg17632 (view) |
Author: mdiaz |
Date: 2014-07-29.13:33:18 |
|
Changes:
- Now we can undo more than one command (for now only
supports record and amend).
1 patch for repository http://darcs.net/screened:
patch 6a88f96435197174ae4154cf796fb41db0a5ba8c
Author: Marcio Diaz <marcio.diaz@gmail.com>
Date: Tue Jul 29 10:29:19 ART 2014
* Implementation of the command 'darcs undo'.
Attachments
|
msg17633 (view) |
Author: ganesh |
Date: 2014-07-29.20:36:49 |
|
Some brief initial comments:
- watch out for spurious whitespace changes
- at some point please merge undoFun into finalizeRepositoryChanges as
discussed before - it'll mean a tedious initial pass through the
codebase marking everything as not undoable by default, but it should
help to shake out any tricky cases and also make sure that nothing is
missed. It'll also be good for future maintainability as people won't be
able to forget undo.
|
msg17634 (view) |
Author: ganesh |
Date: 2014-07-29.20:45:31 |
|
Another general point: this new feature will require substantial testing.
You should start on that now so that you build up the tests as you go, as
it'll be much easier than doing them in one big block at the end. The
tests will also help reviewers understand what the feature can and can't
do.
|
msg17638 (view) |
Author: mdiaz |
Date: 2014-07-30.23:50:20 |
|
Changes:
- Removed spurious whitespace.
- undoFun renamed to 'handleUndoFiles' and called inside finalizeRepositoryChanges.
- Record, unrecord, amend, tag and rebase suspend are marked as YesUndoable.
Looks like rebase unsuspend can't be YesUndoable due to a conditional execution of
finalizeRepositoryChanges in the function checkSuspendedStatus.
1 patch for repository http://darcs.net/screened:
patch e64d722616d62de9d1cd6b645c02c0d74805744f
Author: Marcio Diaz <marcio.diaz@gmail.com>
Date: Wed Jul 30 20:39:21 ART 2014
* Implementation of the command 'darcs undo'.
Attachments
|
msg17645 (view) |
Author: ganesh |
Date: 2014-08-05.18:08:43 |
|
Getting there :-)
Did you document the on-disk format yet? Also don't forget about tests.
I think the most important cleanup left is to sort out the description
of the existing use sites where that's unclear.
One simple change would be to merge the description into the Undoable
type, i.e.
data Undoable = YesUndoable String | NoUndoable
there's no need for a description for a non-undoable command (is there?)
That would eliminate a lot of the existing sites with unclear
descriptions.
It's fine for "rebase453" ("rebase inject") to be NoUndoable - that's a
hacky private command.
I think "rebase obliterate" is undoable, did you have a specific reason
that it isn't?
|
msg17647 (view) |
Author: ganesh |
Date: 2014-08-06.07:40:46 |
|
I had a look at the UI. It's a bit confusing, particularly with the
"Last state" thing. On the other hand, the UI does mean it supports
"redo" implicitly.
I wonder if it would be better to have a separate "redo" command
instead, and present the things to undo (and redo) as a more explicit
stack where the last command is listed first. Perhaps if a single "darcs
undo" could only undo one operation, the UI would be simpler - it would
just say "undo record [y/n]?"
|
msg17655 (view) |
Author: mdiaz |
Date: 2014-08-18.12:07:39 |
|
Taking into account your suggestions I implemented two
separate commands: 'undo' and 'redo'.
Both undo or redo just one operation.
I think you were right, it seems much simpler now.
For now, as indicated by the test, its only enabled for
record, unrecord, amend and obliterate.
For the next version I'll probably move handleUndoFiles
out of the function finalizeRepositoryChanges,
since the revert command doesn't call this function.
Now we are going to support much more commands,
so I wonder what will be the protocol (if any)
to clear the saved states.
2 patches for repository http://darcs.net/screened:
patch c9b040dd1a6d7d0de140beb23f59b9440f00cab5
Author: Marcio Diaz <marcio.diaz@gmail.com>
Date: Mon Aug 18 00:10:31 ART 2014
* Implementation of the commands 'undo' and 'redo'.
patch b6f633d4c9806861a4e5a3f12a064ae10d652a4f
Author: Marcio Diaz <marcio.diaz@gmail.com>
Date: Mon Aug 18 00:11:35 ART 2014
* Test for undo and redo commands.
Attachments
|
msg17657 (view) |
Author: ganesh |
Date: 2014-08-19.22:00:48 |
|
I don't think we need to clear out the saved states explicitly, only when
something invalidates them.
The user doesn't need to use 'darcs undo' if she doesn't want to, so I
don't think they get in the way.
|
msg18019 (view) |
Author: bfrk |
Date: 2015-02-05.15:42:43 |
|
Hm, last activity 6 months ago. Looks like you have sorted out most of
the initial objections, so shouldn't this go into screened? If you don't
have the time I can try to rebase it (guessing that this will be
necessary). There is no milestone yet, but (again guessing) you haven't
planned for this to go into 2.10, or have you?
|
msg18035 (view) |
Author: gh |
Date: 2015-02-05.19:45:59 |
|
I don't think we should take the risk to have this into 2.10 since it is
a new feature, its specification is still not clear, and its author is
no longer around.
|
msg20173 (view) |
Author: ganesh |
Date: 2018-06-24.09:46:13 |
|
Giving up on this,it can be resurrected later if anyone is interested.
|
|
Date |
User |
Action |
Args |
2014-07-23 19:23:31 | mdiaz | create | |
2014-07-29 13:33:18 | mdiaz | set | files:
+ patch-preview.txt, implementation-of-the-command-_darcs-undo__.dpatch, unnamed messages:
+ msg17632 |
2014-07-29 20:36:50 | ganesh | set | status: needs-screening -> in-discussion messages:
+ msg17633 |
2014-07-29 20:45:31 | ganesh | set | messages:
+ msg17634 |
2014-07-29 20:45:34 | ganesh | set | assignedto: ganesh nosy:
+ ganesh |
2014-07-30 23:50:20 | mdiaz | set | files:
+ patch-preview.txt, implementation-of-the-command-_darcs-undo__.dpatch, unnamed messages:
+ msg17638 |
2014-08-05 18:08:44 | ganesh | set | messages:
+ msg17645 |
2014-08-06 07:40:46 | ganesh | set | messages:
+ msg17647 |
2014-08-18 12:07:40 | mdiaz | set | files:
+ patch-preview.txt, implementation-of-the-commands-_undo_-and-_redo__.dpatch, unnamed messages:
+ msg17655 |
2014-08-19 22:00:48 | ganesh | set | messages:
+ msg17657 |
2015-02-05 15:42:44 | bfrk | set | messages:
+ msg18019 |
2015-02-05 19:46:00 | gh | set | messages:
+ msg18035 |
2018-06-24 09:46:13 | ganesh | set | status: in-discussion -> obsoleted messages:
+ msg20173 |
|