On Sat, Apr 11, 2015 at 10:48:22PM -0700, Junio C Hamano wrote: > Max Kirillov <max@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html