Re: [PATCH v2 0/9] diff --color-moved-ws fixes and enhancment

2018-11-27 Thread Phillip Wood

Hi Stefan

On 26/11/2018 21:20, Stefan Beller wrote:

On Fri, Nov 23, 2018 at 3:17 AM Phillip Wood  wrote:


From: Phillip Wood 

Thanks to Stefan for his feedback on v1. I've updated patches 2 & 8 in
response to those comments - see the range-diff below for details (the
patch numbers are off by one in the range diff, I think because the
first patch is unchanged and so it was used as the merge base by
--range-diff=.


`git range-diff` accepts a three dotted "range" OLD...NEW
as an easy abbreviation for the arguments
"COMMON..OLD COMMON..NEW" and the common element is
computed as the last common element. It doesn't have knowledge
about where you started your topic branch.


I was using the new --range-diff option to format-patch, I think I 
should have given --range-diff=@{u}...



For some reason the range-diff also includes
the notes even though I did not give --notes to format-patch)


This is interesting.
The existence of notes.rewrite. seems to work well
with the range-diff then, as the config would trigger the copy-over
of notes and then range-diff would diff the original notes to the new
notes.


Yes, but I think with format-patch it should only diff the notes when 
--notes is given.



When trying out the new --color-moved-ws=allow-indentation-change I
was disappointed to discover it did not work if the indentation
contains a mix of spaces and tabs. This series reworks it so that it
does.



The range-diff looks good to me.


That's good, thanks for your comments on the previous iterations.

Best Wishes

Phillip

Thanks,
Stefan





Re: [PATCH v2 0/9] diff --color-moved-ws fixes and enhancment

2018-11-26 Thread Stefan Beller
On Fri, Nov 23, 2018 at 3:17 AM Phillip Wood  wrote:
>
> From: Phillip Wood 
>
> Thanks to Stefan for his feedback on v1. I've updated patches 2 & 8 in
> response to those comments - see the range-diff below for details (the
> patch numbers are off by one in the range diff, I think because the
> first patch is unchanged and so it was used as the merge base by
> --range-diff=.

`git range-diff` accepts a three dotted "range" OLD...NEW
as an easy abbreviation for the arguments
"COMMON..OLD COMMON..NEW" and the common element is
computed as the last common element. It doesn't have knowledge
about where you started your topic branch.


> For some reason the range-diff also includes
> the notes even though I did not give --notes to format-patch)

This is interesting.
The existence of notes.rewrite. seems to work well
with the range-diff then, as the config would trigger the copy-over
of notes and then range-diff would diff the original notes to the new
notes.

>
> When trying out the new --color-moved-ws=allow-indentation-change I
> was disappointed to discover it did not work if the indentation
> contains a mix of spaces and tabs. This series reworks it so that it
> does.
>

The range-diff looks good to me.

Thanks,
Stefan


[PATCH v2 0/9] diff --color-moved-ws fixes and enhancment

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

Thanks to Stefan for his feedback on v1. I've updated patches 2 & 8 in
response to those comments - see the range-diff below for details (the
patch numbers are off by one in the range diff, I think because the
first patch is unchanged and so it was used as the merge base by
--range-diff=. For some reason the range-diff also includes
the notes even though I did not give --notes to format-patch)

When trying out the new --color-moved-ws=allow-indentation-change I
was disappointed to discover it did not work if the indentation
contains a mix of spaces and tabs. This series reworks it so that it
does.


Phillip Wood (9):
  diff: document --no-color-moved
  Use "whitespace" consistently
  diff: allow --no-color-moved-ws
  diff --color-moved-ws: demonstrate false positives
  diff --color-moved-ws: fix false positives
  diff --color-moved=zebra: be stricter with color alternation
  diff --color-moved-ws: optimize allow-indentation-change
  diff --color-moved-ws: modify allow-indentation-change
  diff --color-moved-ws: handle blank lines

 Documentation/diff-options.txt |  15 ++-
 Documentation/git-cat-file.txt |   8 +-
 diff.c | 219 +
 t/t4015-diff-whitespace.sh |  99 ++-
 4 files changed, 255 insertions(+), 86 deletions(-)

Range-diff against v1:
1:  ae58ae4f29 ! 1:  4939ee371d diff: use whitespace consistently
@@ -1,9 +1,10 @@
 Author: Phillip Wood 
 
-diff: use whitespace consistently
+Use "whitespace" consistently
 
-Most of the documentation uses 'whitespace' rather than 'white space'
-or 'white spaces' convert to latter two to the former for consistency.
+Most of the messages and documentation use 'whitespace' rather than
+'white space' or 'white spaces' convert to latter two to the former for
+consistency.
 
 Signed-off-by: Phillip Wood 
 
@@ -29,6 +30,39 @@
whitespace is the same per line. This is incompatible with the
other modes.
 
+ diff --git a/Documentation/git-cat-file.txt 
b/Documentation/git-cat-file.txt
+ --- a/Documentation/git-cat-file.txt
+ +++ b/Documentation/git-cat-file.txt
+@@
+ stdin, and the SHA-1, type, and size of each object is printed on stdout. 
The
+ output format can be overridden using the optional `` argument. If
+ either `--textconv` or `--filters` was specified, the input is expected to
+-list the object names followed by the path name, separated by a single 
white
+-space, so that the appropriate drivers can be determined.
++list the object names followed by the path name, separated by a single
++whitespace, so that the appropriate drivers can be determined.
+ 
+ OPTIONS
+ ---
+@@
+   Print object information and contents for each object provided
+   on stdin.  May not be combined with any other options or arguments
+   except `--textconv` or `--filters`, in which case the input lines
+-  also need to specify the path, separated by white space.  See the
++  also need to specify the path, separated by whitespace.  See the
+   section `BATCH OUTPUT` below for details.
+ 
+ --batch-check::
+ --batch-check=::
+   Print object information for each object provided on stdin.  May
+   not be combined with any other options or arguments except
+   `--textconv` or `--filters`, in which case the input lines also
+-  need to specify the path, separated by white space.  See the
++  need to specify the path, separated by whitespace.  See the
+   section `BATCH OUTPUT` below for details.
+ 
+ --batch-all-objects::
+
  diff --git a/diff.c b/diff.c
  --- a/diff.c
  +++ b/diff.c
2:  7072bc6211 = 2:  204c7fea9d diff: allow --no-color-moved-ws
3:  ce3ad19eea = 3:  542b79b215 diff --color-moved-ws: demonstrate false 
positives
4:  700e0b61e7 = 4:  4ffb5c4122 diff --color-moved-ws: fix false positives
5:  9ecd8159a7 = 5:  a3a84f90c5 diff --color-moved=zebra: be stricter with 
color alternation
6:  1b1158b1ca = 6:  f94f2e0bae diff --color-moved-ws: optimize 
allow-indentation-change
7:  d8a362be6a ! 7:  fe8eb9cdbc diff --color-moved-ws: modify 
allow-indentation-change
@@ -17,7 +17,7 @@
 
 This commit changes the way the indentation is handled to track the
 visual size of the indentation rather than the characters in the
-indentation. This has they benefit that any whitespace errors do not
+indentation. This has the benefit that any whitespace errors do not
 interfer with the move detection (the whitespace errors will still be
 highlighted according to --ws-error-highlight). During the discussion
 of this feature there were concerns about the correct detection of
@@ -30,7 +30,7 @@
 they are uncolored.
 
 Signed-off-by: Phillip Wood 
-Changes since rfc: