Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes:
> Another patch to test the water before I put more effort into it.
>
> Commit d516c2d (Teach git-diff-files the new option `--no-index` -
> 2007-02-22) brings the bells and whistles of git-diff to the world
> outside a git repository. This patch continues that direction and adds
> a new syntax
>
> git diff --no-index [--] <path> <path> -- <pathspec>
>
> It's easy to guess that the two directories will be filtered by
> pathspec,...
Ugh. Nobody's diff works that way, and nowhere in our UI we use
double-dashes that way, either.
So while I like the idea of "I want to tell Git to compare these two
directories with these pathspec", I do not think we would want to
touch such a syntax with 10 foot pole X-<.
> @@ -194,19 +207,23 @@ void diff_no_index(struct rev_info *revs,
> int j;
> if (!strcmp(argv[i], "--no-index"))
> i++;
> - else if (!strcmp(argv[i], "--"))
> + else if (!strcmp(argv[i], "--")) {
> i++;
> - else {
> + break;
> + } else {
I think this part is a good bugfix regardless of your new feature.
The general command line structure ought to be:
$ git diff [--no-index] [--<diff options>...] [--] <path> <path>
but the existing code is too loose in that "--" is just ignored.
The caller in builtin/diff.c makes sure "--no-index" comes before
"--", the latter of which stops option processing, so it is not so
grave a bug, though.
Coming back to the command line syntax for the new feature, if I had
to choose, I would say
git diff --no-index [-<options>] [--] <path> <path> <pathspec>
perhaps? As we never compare anything other than two things, we can
do the following:
* Implicit --no-index heuristics will kick in _ONLY_ when we have
two things at the end after parsing options in builtin/diff.c, as
before;
* In diff_no_index() after parsing options at its beginning,
- if we have only two things left, they may be
. two files;
. a file and "-" or "-" and a file; or
. two directories (fully traversed without pathspecs)
just as before.
- if we have more than two things left, the first two of these
_MUST_ be directories, and the remainder is pathspec to filter
the result of directory traversal.
Wluldn't that be cleaner and easier to understand?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
