Stefan Beller <stefanbeller@xxxxxxxxxxxxxx> writes:
> The changes in the following patch are in diff_no_index.c, but the
> diff_no_index(...) is called from cmd_diff, which is in builtin/diff.c
> That cmd_diff is actually called from git.c having the
> { "diff", cmd_diff }, entry in handle_internal_command.
>
> My question now is this: Why is the builtin/diff.c relying on stuff
> outside of builtin/ ? Wouldn't it be better to move all these files
> (such as diff_no_index.c) into the builtin folder as well?
Builtins link all sorts of stuff from outside, e.g. diff.c and
diffcore-*.c at the toplevel. I do not see diff_no_index.c is any
different, so I am probably not understanding your question.
> Regarding the removal of the -q option, I tried it in the second patch.
> Is it as easy as that, or am I missing the point?
>
> The first patch doesn't change the behavior, so I'd assume it's safe to
> apply it to origin/sb/misc-fixes, whereas the second patch will make
> git diff complain about the -q option, so I'd assume it would wait for the
> next major release?
>
> Before:
> touch actual_file
> git diff -q actual_file no_file
> error: Could not access 'no_file'
Hmm, do you really get that error message? I think you would get
fatal: ambiguous argument 'no_file': unknown revision or path not in the working tree.
> echo $?
> 1
The command line parsing infrastructure has changed vastly since
"show-diff" days (see below for a history lesson); I think your
"Before" should read more like this
git diff -q -- actual_file no_file
and it should not show removal of no_file in its output. E.g. in
git.git
$ git reset --hard
$ rm COPYING
$ git diff -q -- COPYING
should show nothing.
I personally think "-q" no longer makes sense in today's codebase,
but I am not convinced that removal of '-q' from the proper "git
diff-files" and the "git diff --no-index" (aka "I am too lazy to
teach our diff enhancement to other people's diff implementations,
so let's throw in a "files do not have to be tracked in Git
repository at all" mode") is the right direction to go.
The "-q" option is a remnant from the "show-diff" command, the
precursor of today's "git diff-files" (back then, we didn't even
have "git" potty. The user literally typed "show-diff", not "git
show-diff").
ca2a0798 ([PATCH] Add "-q" option to show-diff.c, 2005-04-15) added
that option. Back then, we did not have pathspec matching, and we
iterated over command line arguments, and required all of them exist
as filesystem entities. "-q" was a way to defeat that "you name a
file, it must exist in the working tree" safety, and also at the
same time not give output for such a file that was removed from the
working tree.
These days, the former "safety" is enforced by the generalized
revision parser ("is it a path or is it a rev?") code and the "--"
delimiter on the command line is the way to defeat it. The latter
is done by giving a filtering specification that lack D to the
"--diff-filter".
If we wanted to make "-q" follow the spirit of its original addition
to "show-diff" again, we could internally add a diff-filter when the
"-q" option is parsed.
"git diff -q ..." is "git diff --diff-filter=ACMRTUB ...", and "git
diff -q --diff-filter=AD" is "git diff --diff-filter=A". That would
let us remove the special case for SILENT_ON_REMOVED, and also make
"-q" work across various commands in the "diff" family. It might
even make it work for "diff --no-index", but I didn't bother to
check.
> After:
> touch actual_file
> git diff -q actual_file no_file
> fatal: invalid diff option/value: -q
> echo $?
> 128
>
> Thanks,
> Stefan
>
> Stefan Beller (2):
> diff --no-index: remove nonfunctional "-q" handling
> git diff: Remove -q option to stay silent on missing files.
>
> Documentation/git-diff-files.txt | 6 +-----
> diff-no-index.c | 5 -----
> 2 files changed, 1 insertion(+), 10 deletions(-)
--
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