Issue 291 darcs record interactive option: "e" is for "edit"

Title darcs record interactive option: "e" is for "edit"
Priority feature Status resolved
Milestone Resolved in
Superseder Nosy List ckeen, darcs-devel, dmitry.kurochkin, galbolle, ganesh, haikuvei, hoelzro, jaredj, kowey, markstos, mornfall, pupeno, thorkilnaur, tommy, zandr, zooko
Assigned To ganesh
Topics Git, UI

Created on 2006-10-12.06:38:16 by kowey, last changed 2010-03-23.16:04:59 by kowey.

File name Uploaded Type Edit Remove
PatchChoices.dpatch ganesh, 2009-08-30.15:55:39 text/x-darcs-patch
msg1066 (view) Author: kowey Date: 2006-10-12.06:35:25
Import of RT #145

markstos <mark@summersault.com> 2004-12-17
Here's my wish for a new option for the interactive darcs record:

option: e
short description: "edit file at this point"
long description:
Selecting this option opens the file being reviewed in an editor,
jumping to the first line of the hunk if possible. Once the editor
exits, the user will taken back to the interactive record session for
that file, starting with the first unapproved hunk in the file. If the
file has changed too much, perhaps will be necessary to start over
asking about hunks in that file.

The editor command invoked would default to 'DARCS_EDITOR +line_num
file_name.txt'. (That syntax works both vi and emacs derivatives).

Perhaps the editor command could be futher configured through an
environmental variable or a preference file, but I'm not sure that's
necessary for a first cut of the feature.

Here's a 'use case' for the feature: I was recording and saw that I had
left in some debugging statements I meant to remove. Whoops!. With the
"e" command, I could jump into the file, remove those lines, and return
to my record session.
msg7755 (view) Author: hoelzro Date: 2009-04-24.19:25:04
Alright, I've been working on this on and off for a while now, so I figure I
should put down a bit of an update:

There's a sort-of working implementation in my repository at
This implementation will allow you to edit a file at a given hunk, and it'll
build a new hunk
list for that file when you're done.  However, the new hunks won't make it into
the recorded
patch, and any old hunks that you may have removed during your edit are
still there (More on this below).  There are more notes about the
limitations of my implementation in the patch's comment section, but they
aren't as bad/difficult to solve as the problem I'm about to discuss.

The reason my implementation hasn't made any progress in the last month is
that I've been trying to figure out how to create a new PatchChoices object
to reflect the new set of hunks.  The problem with this is that if I were to
make a new PatchChoices object, I would lose all of the user's previous
selections, which makes hunk editing pretty much useless.  So for this feature
to progress, one of the following needs to happen:

- I need to be proven wrong in my conception that there's no way to import the
  choices from one PatchChoices object to another.  I base this on the idea
  that a Tag from one PatchChoices object only identifies its associated hunk
  in that PatchChoices object, and not in any other PatchChoices object.

- The PatchChoices facility needs to be retooled so that each hunk has a
  unique ID across all PatchChoices objects.  I feel this could be
  implemented with something like a map, with a hunk's checksum as the key.

Normally, I'd proceed along option #2, but I'd rather have my thoughts
reviewed by more experienced Darcs hackers before I make large code changes.
msg8604 (view) Author: kowey Date: 2009-08-30.15:43:50
I've been chatting with Ganesh about this.  Here are my notes on the problem:

Ps   X   Y   Zs

- 1. break up a hunk patch into smaller patches

- 2. modify such a patch

- 3. maybe: modify the file including stuff not in the patch

- user interface: edit patch or edit file? supply both?
  - edit patch may be easier for wish 1
  - edit file seems simpler in general

- how to deal with subsequent patches
  - suppose I modify X into some X2
  - ideally Y Zs would be adjusted into Y2 Zs2
  - what if Y depends on X?
  - what if Y conflicts with X2 (roughly speaking)
  - scenarios:
     * X <-> Y
       * still X2 <-> Y (eg X2 is subset of X, OK!)
       * now Y conflicts wxth X2
     * not X <-> Y (Y depends on X)
       * now X2 <-> Y (eg X2 is subset of X, OK!)
       * what if X2 introduces a conflict?
  - wishlist
     * if Y is unhappy, skip it and move on to Zs
     * note that there may be unhappy Ys interleaved with Zs (maybe?)

- what do we do with the working directory?
  - if we confirm the record, do we just unapply the original X Y Zs and apply
the new X2 Y2 Zs?
  - what if we hit a conflict?
  - in case of #1 no need to touch working
  - in case of #2 then darcs revert to get rid of

- if Y conflicts with X2: could we refuse the modification?
- if Y depends on X

There are two ways to think about this.  Initially, I was thinking in terms of
merge (hence my concerns about the difficulty porting Y Zs to the new X2 (aka N).

Ganesh was thinking about it in a (imho) much nicer way, which is that the edit
operation always results into two patches: the new patch N and then the
leftovers [coalesce(N^ X)].  NB: to update the working directory, you just
revert the leftovers.

Ps         X  Y   Zs
Ps   N [N^ X] Y   Zs
msg8605 (view) Author: ganesh Date: 2009-08-30.15:55:39
As far as the issue with preserving PatchChoices goes, I think the attached
patch mostly deals with the issue in a fairly non-invasive way by allowing the
Tag structure to be hierarchical so you can keep the old tags for all the other
patches and just start selecting from the new patches introduced during splitting.

I've already submitted the first patch of the sequence as I think it makes sense
in itself, but the second patch should probably only be applied along with code
that actually makes use of it.
msg10456 (view) Author: kowey Date: 2010-03-23.16:04:55
With the UI discussion out of the way, and Darcs 2.4 out the door, it
looks like we can consider this issue resolved

Date User Action Args
2006-10-12 06:38:17koweycreate
2007-08-14 06:25:35koweysettopic: - FromRequestTracker
nosy: + beschmi
2008-02-01 03:31:43markstossetstatus: unread -> deferred
nosy: droundy, tommy, beschmi, kowey, markstos
2008-08-12 15:22:13koweylinkissue989 superseder
2008-08-12 15:22:26koweysetnosy: + dagit, galbolle
2008-08-12 15:24:09koweysettopic: + ProbablyEasy, UI
nosy: + jaredj
2008-08-12 15:47:03koweyunlinkissue989 superseder
2008-11-19 16:13:35ckeensetnosy: + dmitry.kurochkin, simon, thorkilnaur
superseder: + wish: ablity to split and otherwise record a hunk in record
2009-01-10 16:37:11hoelzrosetnosy: + hoelzro
2009-01-11 06:20:30hoelzrosetstatus: deferred -> has-patch
nosy: droundy, tommy, beschmi, kowey, markstos, dagit, simon, thorkilnaur, jaredj, dmitry.kurochkin, galbolle, hoelzro
assignedto: hoelzro
2009-03-22 21:13:00koweylinkissue989 superseder
2009-04-24 19:25:12hoelzrosetnosy: droundy, tommy, beschmi, kowey, markstos, dagit, simon, thorkilnaur, jaredj, dmitry.kurochkin, galbolle, hoelzro
messages: + msg7755
2009-04-28 15:03:55droundysetnosy: - droundy
2009-08-06 20:43:50adminsetnosy: - beschmi
2009-08-10 23:56:41adminsetnosy: - dagit
2009-08-24 01:46:20koweysettopic: - ProbablyEasy
nosy: tommy, kowey, markstos, simon, thorkilnaur, jaredj, dmitry.kurochkin, galbolle, hoelzro
2009-08-25 17:34:42adminsetnosy: + darcs-devel, - simon
2009-08-27 14:31:30adminsetnosy: tommy, kowey, markstos, darcs-devel, thorkilnaur, jaredj, dmitry.kurochkin, galbolle, hoelzro
2009-08-30 15:39:45koweylinkissue126 superseder
2009-08-30 15:43:57koweysettopic: + Git
nosy: + ckeen, zandr, zooko, ganesh, haikuvei, mornfall, pupeno
superseder: - wish: ablity to split and otherwise record a hunk in record
messages: + msg8604
2009-08-30 15:55:41ganeshsetfiles: + PatchChoices.dpatch
nosy: tommy, kowey, markstos, darcs-devel, zooko, ganesh, pupeno, thorkilnaur, jaredj, dmitry.kurochkin, zandr, mornfall, galbolle, ckeen, haikuvei, hoelzro
messages: + msg8605
2009-09-24 07:15:15koweysetnosy: tommy, kowey, markstos, darcs-devel, zooko, ganesh, pupeno, thorkilnaur, jaredj, dmitry.kurochkin, zandr, mornfall, galbolle, ckeen, haikuvei, hoelzro
assignedto: hoelzro -> ganesh
2010-01-18 10:05:16koweylinkissue114 superseder
2010-03-23 16:04:59koweysetstatus: has-patch -> resolved
messages: + msg10456
2010-10-17 14:37:24galbolleunlinkissue114 superseder