darcs

Issue 1742 Need a way to treat common lines as different

Title Need a way to treat common lines as different
Priority feature Status needs-reproduction
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, ganesh, john, kowey
Assigned To
Topics

Created on 2010-02-11.10:01:52 by ganesh, last changed 2010-02-16.11:11:09 by kowey.

Messages
msg9998 (view) Author: ganesh Date: 2010-02-11.10:01:44
---------- Forwarded message ----------
Date: Mon, 8 Feb 2010 22:09:50 -0800
From: John Horigan <john@glyphic.com>
To: darcs-users@darcs.net
Subject: [darcs-users] Can't get hunk editing to do what I want
X-Received-Via: ganesh@earth.li
X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW
     autolearn=no version=3.2.5

I made to changes to my code that are next to each other and part of the same patch, but should be distinct:

     void
     ASTparameterList::entropy(Modification& mod) const
     {
         for (unsigned i = 0; i < mParameters.size(); ++i)
             mod.mRand48Seed ^= mParameters[i].mMod.modData.mRand48Seed;
     }

     ASTparameterList::~ASTparameterList()
     {
         for (std::vector<ASTparameter>::iterator it = mParameters.begin();
              it != mParameters.end(); ++it)
         {
             delete (*it).mExpression;
             for (ASTexpArray::iterator it2 = (*it).mMod.modExp.begin(); it2 != (*it).mMod.modExp.end(); ++it2)
                 delete (*it2);
             (*it).mMod.modExp.clear();
         }
     }

became

     void
     ASTparameterList::entropy(Modification& mod) const
     {
         for (ASTparameters::const_iterator it = mParameters.begin();
              it != mParameters.end(); ++it)
         {
             mod.mRand48Seed ^= (*it).mMod.modData.mRand48Seed;
         }
     }

     ASTparameter::~ASTparameter()
     {
         delete mExpression;
     }


So I changed the loop in the entropy method and replaced the ASTparameterList destructor with an ASTparameter destructor. Darcs mixes together all of these changes into three hunks:

hunk ./src-common/astreplacement.cpp 124
-        for (unsigned i = 0; i < mParameters.size(); ++i)
-            mod.mRand48Seed ^= mParameters[i].mMod.modData.mRand48Seed;
-    }
-    $
-    ASTparameterList::~ASTparameterList()
-    {
-        for (std::vector<ASTparameter>::iterator it = mParameters.begin();
+        for (ASTparameters::const_iterator it = mParameters.begin(); $
Shall I record this change? (8/495)  [ynWesfvplxdaqjk], or ? for help: y
hunk ./src-common/astreplacement.cpp 127
-            delete (*it).mExpression;
-            for (ASTexpArray::iterator it2 = (*it).mMod.modExp.begin(); it2 != (*it).mMod.modExp.end(); ++it2)
-                delete (*it2);
-            (*it).mMod.modExp.clear();
+            mod.mRand48Seed ^= (*it).mMod.modData.mRand48Seed;
Shall I record this change? (9/495)  [ynWesfvplxdaqjk], or ? for help: y
hunk ./src-common/astreplacement.cpp 131
+    ASTparameter::~ASTparameter()
+    {
+        delete mExpression;
+    }
+    $
Shall I record this change? (10/495)  [ynWesfvplxdaqjk], or ? for help: y

Which is a mess. The hunks I would like to record are:

-        for (unsigned i = 0; i < mParameters.size(); ++i)
-            mod.mRand48Seed ^= mParameters[i].mMod.modData.mRand48Seed;
+        for (ASTparameters::const_iterator it = mParameters.begin();
+             it != mParameters.end(); ++it)
+        {
+            mod.mRand48Seed ^= (*it).mMod.modData.mRand48Seed;
+        }

and

-    ASTparameterList::~ASTparameterList()
-    {
-        for (std::vector<ASTparameter>::iterator it = mParameters.begin();
-             it != mParameters.end(); ++it)
-        {
-            delete (*it).mExpression;
-            for (ASTexpArray::iterator it2 = (*it).mMod.modExp.begin(); it2 != (*it).mMod.modExp.end(); ++it2)
-                delete (*it2);
-            (*it).mMod.modExp.clear();
-        }
+    ASTparameter::~ASTparameter()
+    {
+        delete mExpression;

I tried various things with the hunk editor to get darcs to reduce the scope of the first hunk but to no avail. This isn't a case of splitting an add or a delete. Darcs sees the lines

              it != mParameters.end(); ++it)
         {

in pristine and working and came up with hunks that preserved those lines. This is pretty cool behavior, but it does not really represent the change that I made. I want darcs to rethink the hunk based on a smaller piece of the pristine file.  In the end, I changed the name of the loop variable so that the spurious common lines between pristine and working went away. Am I expecting too much from hunk editing? Is there some way to get darcs to ignore spurious common lines between pristine and working?

-- john

_______________________________________________
darcs-users mailing list
darcs-users@darcs.net
http://lists.osuosl.org/mailman/listinfo/darcs-users
msg9999 (view) Author: ganesh Date: 2010-02-11.10:51:13
On Mon, 8 Feb 2010, John Horigan wrote:

> Am I expecting too much from hunk editing? Is there some way to get 
> darcs to ignore spurious common lines between pristine and working?

Thanks for the feedback - this is a use case I hadn't considered, and it's 
not something that hunk editing in its current form can handle. Thinking 
about it, it's something I've also occasionally encountered, though 
nowhere near as frequently as things like wanting to tease apart two 
separate changes or clean up some noise in a patch.

I think what's needed here is some way to ask darcs to merge two hunks, 
and then use the hunk editor to split them where you want.

The tricky bit here will be preventing it from turning the end result back 
into the original, because there's a "canonization" pass that normally 
runs which will have this effect. Normally that's a good thing because it 
produces minimal changes, but here it's working against you.

A possibly related use case is when darcs detects a new function as 
including the closing curly brace of the previous function instead of its 
own closing curly brace.

I've opened a ticket - http://bugs.darcs.net/issue1742

Cheers,

Ganesh
msg10009 (view) Author: kowey Date: 2010-02-16.11:11:07
Marking need-action for UI discussion/design.

For tracking purposes, I've added a wiki page collecting some of the
fancier things we'd like to be able to do with darcs record:
http://wiki.darcs.net/Ideas/FancyRecording
History
Date User Action Args
2010-02-11 10:01:52ganeshcreate
2010-02-11 10:47:41ganeshsettitle: Can't get hunk editing to do what I want (fwd) -> Need a way to treat common lines as different
2010-02-11 10:51:15ganeshsetnosy: + darcs-users, john
messages: + msg9999
2010-02-16 11:11:09koweysetpriority: feature
status: unknown -> needs-reproduction
messages: + msg10009
nosy: + kowey, - darcs-users