Re: [Classpath] Re: code indentation

[C. Scott Ananian]

On 28 Sep 2002, Nic Ferrier wrote:

> You're changes are unlikely to be accepted if you mix style and
> logical changes.

But, luckily, 'cvs diff -U1' can come to the rescue.  Do:

$ cvs diff -U1 some-directory-with-modified-files > my-changes

Now open my-changes in a text editor.  You can remove any section from a
'@@' to another '@@' without corrupting the diff.  The idea is to remove
all sections that correspond to 'real' changes, while leaving all the
sections that correspond to formatting-only changes.  For example:
---- start file 'my-changes' ----
Index: java/util/Date.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/Date.java,v
retrieving revision 1.14
diff -u -p -U1 -r1.14 Date.java
--- java/util/Date.java 22 Jan 2002 22:27:01 -0000      1.14
+++ java/util/Date.java 29 Sep 2002 05:19:54 -0000
@@ -76,3 +76,3 @@ public class Date implements Cloneable, 
   {
-    time = System.currentTimeMillis();
+      time = System.currentTimeMillis();
   }
@@ -85,3 +85,3 @@ public class Date implements Cloneable, 
   {
-    this.time = time;
+    this.time = time * 2;
   }
@@ -105,5 +105,4 @@ public class Date implements Cloneable, 
   {
-    time =
-      new GregorianCalendar(year + 1900, month, day, hour,
-                           min).getTimeInMillis();
+    time = new GregorianCalendar(year + 1900, month, day, hour,
+                                min).getTimeInMillis();
   }
---- end file 'my-changes' ----

The first section is just a white space change.  You want to leave that
one in.  The second section is a 'real' change (it's actually bogus, in
that the original code was correct, but I had to change something in order
to make this a good example).  And the last section is another
formatting-only change, which you want to remove.  You end up with:
---- start file 'my-changes' ----
Index: java/util/Date.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/Date.java,v
retrieving revision 1.14
diff -u -p -U1 -r1.14 Date.java
--- java/util/Date.java 22 Jan 2002 22:27:01 -0000      1.14
+++ java/util/Date.java 29 Sep 2002 05:19:54 -0000
@@ -76,3 +76,3 @@ public class Date implements Cloneable, 
   {
-    time = System.currentTimeMillis();
+      time = System.currentTimeMillis();
   }
@@ -105,5 +105,4 @@ public class Date implements Cloneable, 
   {
-    time =
-      new GregorianCalendar(year + 1900, month, day, hour,
-                           min).getTimeInMillis();
+    time = new GregorianCalendar(year + 1900, month, day, hour,
+                                min).getTimeInMillis();
   }
---- end file 'my-changes' ----

Note how the second modified section was removed from the file.  Now you
can 'undo' all the changes left in the my-changes file (the changes
corresponding to formatting only) with the slightly-magic command:

$ POSIXLY_CORRECT=1 patch -p0 -R < my-changes

Now a cvs-diff should only show 'good' changes.  Of course sometimes
your good and bad changes are too close together and you can't separate
them cleanly using the above technique, but it should get rid of the large
portion, leaving less for you to manually clean.  Keep using 'cvs diff'
until the only changes shown are 'good' functional (not 'bad' formatting)
changes.  Extra hint --- if you keep your 'my-changes' file, you can
submit it separately as a formatting-only patch -- or reapply it once your
'real code' patch has been accepted with:

$ POSIXLY_CORRECT=1 patch -p0 < my-changes

Hope this helps you clean up your patches for submission!
 --scott

[reading the info pages for diff and patch will help a lot if this
 step-by-step seemed overly magical. =)  I believe that if your version
 of 'cvs' is new enough (1.11.2 works for me) you don't need the
 POSIXLY_CORRECT=1 bit on the patch command.  But old cvs servers (1.10.2,
 for example) leave off the directory part of the file name on the '---'
 and '+++' lines, confusing patch unless you use POSIXLY_CORRECT.]

Indonesia shortwave D5 SLBM Dictionary United Nations LA Nader agent 
assassinate Echelon kibo Iraq security Treasury Attache NORAD fissionable 
                         ( http://cscott.net/ )