Re: [PATCHv5 00/17] Diff machine: highlight moved lines.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I was trying to see how this is useful in code moving change, with
this command line:

$ git -c color.moved diff js/blame-lib~6 js/blame-lib blame.c blame.h builtin/blame.c

Some random observations:

 * You do not seem to have any command line option yet.  I guess
   that is OK while the series is still in RFC state, but when we
   are ready to seriously start considering this for 'next', we'd
   need something like --color-moved that can be used across "diff",
   "log -p" and "show".

 * As configuration variable names go, "color.moved" is probably in
   a wrong section.  Perhaps "diff.colorMoved" or something?

 * The fact that I am using

     [diff "color"] 
	old = red reverse
	whitespace = blue reverse

   on a "black ink on white paper" terminal might have an effect on
   this, but lines printed in either bold green and on green
   background (i.e. not new ones but are the ones moved from
   elsewhere) stood out a lot more prominently than lines printed in
   green (i.e. truly new additions), and I felt that this is totally
   backwards from what I needed for this exercise.  That is, while
   reviewing a code moving change, I want to be able to concentrate
   primarily of three things:

   - What are the new lines that are *not* moved from elsewhere?
     Did the submitter try to sneak in unrelated changes?

   - What are the changes that are truly lost, not moved to
     elsewhere?

   - Do the lines moved from elsewhere form a coherent whole?  Where
     is the boundary between blocks of text that are copied?  Do
     adjacent two blocks of moved text come from the same old place
     next to each other?

   Using colors that stand out more prominently than for the regular
   new/old lines is counter-productive for all of these, especially
   for the first two purposes.  I may suggest using cyan (or any
   color that is very close to the background) so that the presense
   of moved lines are merely felt without being distracting.  IOW,
   while reviewing code moving patch, moved parts want to be dimmed,
   not highlighted.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]