Re: [PATCH 0/2] git diff -q option removal

Stefan Beller [off-list ref] 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(-)

On Wed, Jul 17, 2013 at 10:04 AM, Junio C Hamano [off-list ref] wrote:

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.

Having said all that, I do not mean to advocate to retain "-q". Most
likely nobody uses it, and "-q" is grossly misnamed ("why is it so
special to be "quiet" only for removals?"). As long as we mention its
removal in the release notes (and possibly tell those miniscule
minority that wants to ignore deleted files to use --diff-filter
instead), we should be OK.

Independently, we might want to enhance --diff-filter parser to make
it easier to say "I want everything but D", perhaps use lowercase
letter to subtract from what have been specified so far (or if there
is no uppercase letter, start from "everything"), so that we can say
--diff-filter=d
or something.

On 07/17/2013 07:04 PM, Junio C Hamano wrote:

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.

Thanks for the explanation. I am not yet very used to gits code structure. So I sometimes think of 'how would I structure things', so I
was confused of things in builtin using some parts outside of it.
Maybe that folder raised to much expectations for me to be 'the real'
core, whereas the files outside, i.e. those files in the top level
directory, are just there for other things or scripts.
This understanding of the structure seems obviously wrong now.

Thanks for clarification.

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.

quoted

	echo $?
	1

Ok here we go (using current origin/master 9c3c367):

	cd $(mktemp -d)
	echo "test" > actual_file
	git diff actual_file no_file
	# error: Could not access 'no_file'
	echo $?
	1
	
	## I get the same message as well, if I'm using -- or not.
	## also the -q doesn't make a change

	git init 
	git diff -q -- actual_file no_file
	echo $?
	# 0
	git diff  -- actual_file no_file
	echo $?
	# 0
	git diff  actual_file no_file
	# fatal: no_file: no such path in the working tree.
	# Use 'git <command> -- <path>...' to specify paths that do not exist locally.
	echo $?
	# 128
	git diff -q  actual_file no_file
	# fatal: no_file: no such path in the working tree.
	# Use 'git <command> -- <path>...' to specify paths that do not exist locally.
	echo $?
	128

So apparently git diff behaves differently if not in a repo, which is what I tested.

The command line parsing infrastructure has changed vastly since
"show-diff" days (see below for a history lesson);

A very interesting read, much appreciated. :)

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.

I'm rather new to the project, so my opinion may not have much weight,
I'll state it anyway:
Keeping backwards compatibility is really hard, because you need the 
knowledge of such history lessons as read above, to understand what should
happen, like having an intuitive feeling about such parameters. Hence
maintaining/evolving the project will become harder and harder 
(specially for newcomers). So having one and only one way to achieve the desired
output, which fits into the greater structure as it's the case with --diff-filter
is easier to remember for both the user and developers.

Hence I think keeping the -q option would only make sense for the plumber 
layer, because there the explicit promise was given to not change stuff
every other release. 

Stefan

The --diff-filter=<arg> option given by the user is kept as a
string, and passed to the underlying diffcore_apply_filter()
function as a string for each resulting path we run number of
strchr() to see if each class of change among ACDMRTXUB is meant to
be given.

Change the function signature to pass the whole diff_options, so
that we can pre-parse this string in the next patch.

Signed-off-by: Junio C Hamano <redacted>
---
 diff.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 649ec86..41c64f2 100644
--- a/diff.c
+++ b/diff.c

@@ -4509,11 +4509,13 @@ free_queue:
 	}
 }
 
-static void diffcore_apply_filter(const char *filter)
+static void diffcore_apply_filter(struct diff_options *options)
 {
 	int i;
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct diff_queue_struct outq;
+	const char *filter = options->filter;
+
 	DIFF_QUEUE_CLEAR(&outq);
 
 	if (!filter)

@@ -4661,7 +4663,7 @@ void diffcore_std(struct diff_options *options)
 	if (!options->found_follow)
 		/* See try_to_follow_renames() in tree-diff.c */
 		diff_resolve_rename_copy();
-	diffcore_apply_filter(options->filter);
+	diffcore_apply_filter(options);
 
 	if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
 		DIFF_OPT_SET(options, HAS_CHANGES);

-- 
1.8.3.3-962-gf04df43

The "-q" option given to "git diff-files" is a remnant of 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 command line parsing infrastructure has changed
vastly since "show-diff" days, and 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", e.g. "--diff-filter=ACMRTUB".

This is however a bit cumbersome to type.  This miniseries updates
the diff-filter mechanism to let you say --diff-filter=d (lowercase)
to express that you are interested in the changes in general, but
not the changes in the 'D' class (i.e. deletion).

The last step tweaks the command line parser of "git diff-files"
(and "git diff" without any object on the command line, which goes
to the same codepath) and "git diff --no-index" to notice "-q", warn
and then turn it into "--diff-filter=d".  We should remove the
entire thing at a major version bump, like Git 2.0.


This is still a bit rough, without any documentation updates nor
tests.


Junio C Hamano (6):
  diff: pass the whole diff_options to diffcore_apply_filter()
  diff: factor out match_filter()
  diff: preparse --diff-filter string argument
  diff: reject unknown change class given to --diff-filter
  diff: allow lowercase letter to specify what change class to exclude
  diff: deprecate -q option to diff-files

 diff-lib.c      |   8 ++--
 diff-no-index.c |   7 +++-
 diff.c          | 125 ++++++++++++++++++++++++++++++++++++++++++++++----------
 diff.h          |   7 +++-
 4 files changed, 118 insertions(+), 29 deletions(-)

-- 
1.8.3.3-962-gf04df43

This reimplements the ancient "-q" option to "git diff-files" that
was inherited from "show-diff -q" in terms of "--diff-filter=d", and
issue a warning against the use of the former.

Incidentally this also tentatively fix "git diff --no-index" to
honor "-q" and hide deletions; the use will get the same warning.

We should remove the support for "-q" in Git 2.0.

Signed-off-by: Junio C Hamano <redacted>
---
 diff-lib.c      | 8 +++-----
 diff-no-index.c | 7 +++++--
 diff.c          | 8 ++++++++
 diff.h          | 2 ++
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..4634b29 100644
--- a/diff-lib.c
+++ b/diff-lib.c

@@ -86,10 +86,12 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 {
 	int entries, i;
 	int diff_unmerged_stage = revs->max_count;
-	int silent_on_removed = option & DIFF_SILENT_ON_REMOVED;
 	unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
 			      ? CE_MATCH_RACY_IS_DIRTY : 0);
 
+	if (option & DIFF_SILENT_ON_REMOVED)
+		handle_deprecated_show_diff_q(&revs->diffopt);
+
 	diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
 
 	if (diff_unmerged_stage < 0)

@@ -136,8 +138,6 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 					perror(ce->name);
 					continue;
 				}
-				if (silent_on_removed)
-					continue;
 				wt_mode = 0;
 			}
 			dpath->mode = wt_mode;

@@ -203,8 +203,6 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 				perror(ce->name);
 				continue;
 			}
-			if (silent_on_removed)
-				continue;
 			diff_addremove(&revs->diffopt, '-', ce->ce_mode,
 				       ce->sha1, !is_null_sha1(ce->sha1),
 				       ce->name, 0);

diff --git a/diff-no-index.c b/diff-no-index.c
index 74da659..a788a5f 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c

@@ -187,7 +187,7 @@ void diff_no_index(struct rev_info *revs,
 {
 	int i, prefixlen;
 	int no_index = 0;
-	unsigned options = 0;
+	unsigned deprecated_show_diff_q_option_used = 0;
 	const char *paths[2];
 
 	/* Were we asked to do --no-index explicitly? */

@@ -225,7 +225,7 @@ void diff_no_index(struct rev_info *revs,
 		if (!strcmp(argv[i], "--no-index"))
 			i++;
 		else if (!strcmp(argv[i], "-q")) {
-			options |= DIFF_SILENT_ON_REMOVED;
+			deprecated_show_diff_q_option_used = 1;
 			i++;
 		}
 		else if (!strcmp(argv[i], "--"))

@@ -260,6 +260,9 @@ void diff_no_index(struct rev_info *revs,
 	revs->max_count = -2;
 	diff_setup_done(&revs->diffopt);
 
+	if (deprecated_show_diff_q_option_used)
+		handle_deprecated_show_diff_q(&revs->diffopt);
+
 	setup_diff_pager(&revs->diffopt);
 	DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
 

diff --git a/diff.c b/diff.c
index 2d0b5e3..78819ba 100644
--- a/diff.c
+++ b/diff.c

@@ -3570,6 +3570,14 @@ static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)
 	return 0;
 }
 
+/* Used only by "diff-files" and "diff --no-index" */
+void handle_deprecated_show_diff_q(struct diff_options *opt)
+{
+	warning("'diff -q' and 'diff-files -q' are deprecated.");
+	warning("Use 'diff --diff-filter=d' instead to ignore deleted filepairs.");
+	parse_diff_filter_opt("d", opt);
+}
+
 int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
 	const char *arg = av[0];

diff --git a/diff.h b/diff.h
index a367207..5237d63 100644
--- a/diff.h
+++ b/diff.h

@@ -341,6 +341,8 @@ extern int parse_rename_score(const char **cp_p);
 
 extern long parse_algorithm_value(const char *value);
 
+extern void handle_deprecated_show_diff_q(struct diff_options *);
+
 extern int print_stat_summary(FILE *fp, int files,
 			      int insertions, int deletions);
 extern void setup_diff_pager(struct diff_options *);

-- 
1.8.3.3-962-gf04df43

diffcore_apply_filter() checks if a filepair matches the filter
given with the "--diff-filter" option for each input filepairs with
a fairly complex expression in two places.

Create a helper function and call it.

Signed-off-by: Junio C Hamano <redacted>
---
 diff.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/diff.c b/diff.c
index 41c64f2..0220c19 100644
--- a/diff.c
+++ b/diff.c

@@ -4509,6 +4509,17 @@ free_queue:
 	}
 }
 
+static int match_filter(const struct diff_options *options, const struct diff_filepair *p)
+{
+	return (((p->status == DIFF_STATUS_MODIFIED) &&
+		 ((p->score &&
+		   strchr(options->filter, DIFF_STATUS_FILTER_BROKEN)) ||
+		  (!p->score &&
+		   strchr(options->filter, DIFF_STATUS_MODIFIED)))) ||
+		((p->status != DIFF_STATUS_MODIFIED) &&
+		 strchr(options->filter, p->status)));
+}
+
 static void diffcore_apply_filter(struct diff_options *options)
 {
 	int i;

@@ -4524,14 +4535,7 @@ static void diffcore_apply_filter(struct diff_options *options)
 	if (strchr(filter, DIFF_STATUS_FILTER_AON)) {
 		int found;
 		for (i = found = 0; !found && i < q->nr; i++) {
-			struct diff_filepair *p = q->queue[i];
-			if (((p->status == DIFF_STATUS_MODIFIED) &&
-			     ((p->score &&
-			       strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
-			      (!p->score &&
-			       strchr(filter, DIFF_STATUS_MODIFIED)))) ||
-			    ((p->status != DIFF_STATUS_MODIFIED) &&
-			     strchr(filter, p->status)))
+			if (match_filter(options, q->queue[i]))
 				found++;
 		}
 		if (found)

@@ -4549,14 +4553,7 @@ static void diffcore_apply_filter(struct diff_options *options)
 		/* Only the matching ones */
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
-
-			if (((p->status == DIFF_STATUS_MODIFIED) &&
-			     ((p->score &&
-			       strchr(filter, DIFF_STATUS_FILTER_BROKEN)) ||
-			      (!p->score &&
-			       strchr(filter, DIFF_STATUS_MODIFIED)))) ||
-			    ((p->status != DIFF_STATUS_MODIFIED) &&
-			     strchr(filter, p->status)))
+			if (match_filter(options, p))
 				diff_q(&outq, p);
 			else
 				diff_free_filepair(p);

-- 
1.8.3.3-962-gf04df43

We used to accept "git diff --diff-filter=Q" (note that there is no
such change class 'Q') silently and showed no output (because there
is no such change class 'Q').

Error out when such an input is given.

Signed-off-by: Junio C Hamano <redacted>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 03f10e6..3d37b56 100644
--- a/diff.c
+++ b/diff.c

@@ -3537,7 +3537,7 @@ static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)
 
 		bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;
 		if (!bit)
-			continue; /* ignore unknown ones, like we always have */
+			return optarg[i];
 		opt->filter |= bit;
 	}
 	return 0;

-- 
1.8.3.3-962-gf04df43

Instead of running strchr() on the list of status characters over
and over again, parse the --diff-filter option into bitfields and
use the bits to see if the change to the filepair matches the status
requested.

Signed-off-by: Junio C Hamano <redacted>
---
 diff.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 diff.h |  5 ++++-
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 0220c19..03f10e6 100644
--- a/diff.c
+++ b/diff.c

@@ -3496,6 +3496,53 @@ static int parse_submodule_opt(struct diff_options *options, const char *value)
 	return 1;
 }
 
+static const char diff_status_letters[] = {
+	DIFF_STATUS_ADDED,
+	DIFF_STATUS_COPIED,
+	DIFF_STATUS_DELETED,
+	DIFF_STATUS_MODIFIED,
+	DIFF_STATUS_RENAMED,
+	DIFF_STATUS_TYPE_CHANGED,
+	DIFF_STATUS_UNKNOWN,
+	DIFF_STATUS_UNMERGED,
+	DIFF_STATUS_FILTER_AON,
+	DIFF_STATUS_FILTER_BROKEN,
+	'\0',
+};
+
+static unsigned int filter_bit['Z' + 1];
+
+static void prepare_filter_bits(void)
+{
+	int i;
+
+	if (!filter_bit[DIFF_STATUS_ADDED]) {
+		for (i = 0; diff_status_letters[i]; i++)
+			filter_bit[(int) diff_status_letters[i]] = (1 << i);
+	}
+}
+
+static unsigned filter_bit_tst(char status, const struct diff_options *opt)
+{
+	return opt->filter & filter_bit[(int) status];
+}
+
+static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)
+{
+	int i, optch;
+
+	prepare_filter_bits();
+	for (i = 0; (optch = optarg[i]) != '\0'; i++) {
+		unsigned int bit;
+
+		bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;
+		if (!bit)
+			continue; /* ignore unknown ones, like we always have */
+		opt->filter |= bit;
+	}
+	return 0;
+}
+
 int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
 	const char *arg = av[0];

@@ -3717,7 +3764,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		return argcount;
 	}
 	else if ((argcount = parse_long_opt("diff-filter", av, &optarg))) {
-		options->filter = optarg;
+		int offending = parse_diff_filter_opt(optarg, options);
+		if (offending)
+			die("unknown change class '%c' in --diff-filter=%s",
+			    offending, optarg);
 		return argcount;
 	}
 	else if (!strcmp(arg, "--abbrev"))

@@ -4513,11 +4563,11 @@ static int match_filter(const struct diff_options *options, const struct diff_fi
 {
 	return (((p->status == DIFF_STATUS_MODIFIED) &&
 		 ((p->score &&
-		   strchr(options->filter, DIFF_STATUS_FILTER_BROKEN)) ||
+		   filter_bit_tst(DIFF_STATUS_FILTER_BROKEN, options)) ||
 		  (!p->score &&
-		   strchr(options->filter, DIFF_STATUS_MODIFIED)))) ||
+		   filter_bit_tst(DIFF_STATUS_MODIFIED, options)))) ||
 		((p->status != DIFF_STATUS_MODIFIED) &&
-		 strchr(options->filter, p->status)));
+		 filter_bit_tst(p->status, options)));
 }
 
 static void diffcore_apply_filter(struct diff_options *options)

@@ -4525,14 +4575,13 @@ static void diffcore_apply_filter(struct diff_options *options)
 	int i;
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct diff_queue_struct outq;
-	const char *filter = options->filter;
 
 	DIFF_QUEUE_CLEAR(&outq);
 
-	if (!filter)
+	if (!options->filter)
 		return;
 
-	if (strchr(filter, DIFF_STATUS_FILTER_AON)) {
+	if (filter_bit_tst(DIFF_STATUS_FILTER_AON, options)) {
 		int found;
 		for (i = found = 0; !found && i < q->nr; i++) {
 			if (match_filter(options, q->queue[i]))

diff --git a/diff.h b/diff.h
index 78b4091..a367207 100644
--- a/diff.h
+++ b/diff.h

@@ -103,12 +103,15 @@ enum diff_words_type {
 };
 
 struct diff_options {
-	const char *filter;
 	const char *orderfile;
 	const char *pickaxe;
 	const char *single_follow;
 	const char *a_prefix, *b_prefix;
 	unsigned flags;
+
+	/* diff-filter bits */
+	unsigned int filter;
+
 	int use_color;
 	int context;
 	int interhunkcontext;

-- 
1.8.3.3-962-gf04df43

In order to express "we do not care about deletions", we had to say
"--diff-filter=ACMRTXUB", giving all the possible change class
except for the one we do not want, "D".

This is cumbersome.  As all the change classes are in uppercase,
allow their lowercase counterpart to selectively exclude the class
from the output.  When such a negated change class is in the input,
start the filter option with the full bits set.

This would allow us to express the old "show-diff -q" with
"git diff-files --diff-filter=d".

Signed-off-by: Junio C Hamano <redacted>
---
 diff.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 3d37b56..2d0b5e3 100644
--- a/diff.c
+++ b/diff.c

@@ -3532,13 +3532,40 @@ static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)
 	int i, optch;
 
 	prepare_filter_bits();
+
+	/*
+	 * If there is a negation e.g. 'd' in the input, and we haven't
+	 * initialized the filter field with another --diff-filter, start
+	 * from full set of bits, except for AON.
+	 */
+	if (!opt->filter) {
+		for (i = 0; (optch = optarg[i]) != '\0'; i++) {
+			if (optch < 'a' || 'z' < optch)
+				continue;
+			opt->filter = (1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1;
+			opt->filter &= ~filter_bit[DIFF_STATUS_FILTER_AON];
+			break;
+		}
+	}
+
 	for (i = 0; (optch = optarg[i]) != '\0'; i++) {
 		unsigned int bit;
+		int negate;
+
+		if ('a' <= optch && optch <= 'z') {
+			negate = 1;
+			optch = toupper(optch);
+		} else {
+			negate = 0;
+		}
 
 		bit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;
 		if (!bit)
 			return optarg[i];
-		opt->filter |= bit;
+		if (negate)
+			opt->filter &= ~bit;
+		else
+			opt->filter |= bit;
 	}
 	return 0;
 }

-- 
1.8.3.3-962-gf04df43

Junio C Hamano wrote:

We should remove the support for "-q" in Git 2.0.

Nooooo.  I hope you are teasing.

I don't mind seeing support for "-q" dropped, but I really don't think
it's worth delaying git 2.0 for that.  Would s/in Git 2.0/in some
future release/ be ok?

The patch text itself looks good.

Thanks,
Jonathan