On Sat, Apr 11, 2015 at 10:48:22PM -0700, Junio C Hamano wrote:
> Max Kirillov <[email protected]> writes:
>> Rather, it should verify that file
>> selection behaves the same as hunk selection.
>
> Hrm, really? "git diff --raw" and "git diff -p -w" on two trees
> would not show identical set of paths, when the blob object are
> different byte-wise but are equivalent at the content level per
> given definition of equivalence (e.g. "-w"). Given that --cc is
> to look at the differences at hunk/content level to combine and omit
> uninteresting ones from the output, relative to -c output, I would
> imagine that the file selection and the hunk selection are expected
> to behave differently.
I mean, that if a hunk is shown in some long file, which
have other changes, with some option (-c or --cc), it should
also be shown if it is the only change in file.
> So, having said that I am a bit skeptical about the description of
> the goals, there are a few nits on the implementation, too.
>
> > +# the difference in short file must be returned if and only if it is shown
> > in long file
> > +for fn in win1 win2 merge delete base only1 only2 only1discard
> > only2discard; do
> > + if git diff --cc merge branch1 branch2 mergebase -- long/$fn | grep -q
> > '^[ +-]\{3\}2\(change[12]|merge\)\?$'
> > + then
>
> Just like the earlier parts of this patch, write a newline when you
> do not have to write a semicolon, and split lines after pipe when
> your pipeline gets long, i.e.
>
> for fn in win2 win2 ...
> do
> if git diff --cc ... |
> grep 'pattern'
> then
>
> Do I smell some GNUism in your "grep" patterns?
>
> You have
>
> '^[ +-]\{3\}2\(change[12]|merge\)\?$'
>
> but matching zero-or-one repetition with ? is not in BRE, and \? to
> use it in BRE is a GNU extension.
>
> Also in BRE , '|' is not an alternation (and making it into such
> with '\|' in BRE is a GNU extension IIRC.
>
> Worse, you are not using backslash here to invoke GNU extension, so
> I suspect the grep invocations in the patch may not be working as
> you expect.
Thank you, I missed it. I suspect it was working because
single "2" still matching. Will fix it and other issues.
--
Max
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
