diff-filter: be more careful when looking for negative bits
The `--diff-filter=<bits>` option allows to filter the diff by certain criteria, for example `R` to only show renamed files. It also supports negating a filter via a down-cased letter, i.e. `r` to show _everything but_ renamed files. However, the code is a bit overzealous when trying to figure out whether `git diff` should start with all diff-filters turned on because the user provided a lower-case letter: if the `--diff-filter` argument starts with an upper-case letter, we must not start with all bits turned on. Even worse, it is possible to specify the diff filters in multiple, separate options, e.g. `--diff-filter=AM [...] --diff-filter=m`. Let's accumulate the include/exclude filters independently, and only special-case the "only exclude filters were specified" case after parsing the options altogether. Note: The code replaced by this commit took pains to avoid setting any unused bits of `options->filter`. That was unnecessary, though, as all accesses happen via the `filter_bit_tst()` function using specific bits, and setting the unused bits has no effect. Therefore, we can simplify the code by using `~0` (or in this instance, `~<unwanted-bit>`). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>maint
							parent
							
								
									4d4d4eaa7b
								
							
						
					
					
						commit
						75408ca949
					
				
							
								
								
									
										23
									
								
								diff.c
								
								
								
								
							
							
						
						
									
										23
									
								
								diff.c
								
								
								
								
							|  | @ -4720,6 +4720,12 @@ void diff_setup_done(struct diff_options *options) | ||||||
| 	if (!options->use_color || external_diff()) | 	if (!options->use_color || external_diff()) | ||||||
| 		options->color_moved = 0; | 		options->color_moved = 0; | ||||||
|  |  | ||||||
|  | 	if (options->filter_not) { | ||||||
|  | 		if (!options->filter) | ||||||
|  | 			options->filter = ~filter_bit[DIFF_STATUS_FILTER_AON]; | ||||||
|  | 		options->filter &= ~options->filter_not; | ||||||
|  | 	} | ||||||
|  |  | ||||||
| 	FREE_AND_NULL(options->parseopts); | 	FREE_AND_NULL(options->parseopts); | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -4820,21 +4826,6 @@ static int diff_opt_diff_filter(const struct option *option, | ||||||
| 	BUG_ON_OPT_NEG(unset); | 	BUG_ON_OPT_NEG(unset); | ||||||
| 	prepare_filter_bits(); | 	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++) { | 	for (i = 0; (optch = optarg[i]) != '\0'; i++) { | ||||||
| 		unsigned int bit; | 		unsigned int bit; | ||||||
| 		int negate; | 		int negate; | ||||||
|  | @ -4851,7 +4842,7 @@ static int diff_opt_diff_filter(const struct option *option, | ||||||
| 			return error(_("unknown change class '%c' in --diff-filter=%s"), | 			return error(_("unknown change class '%c' in --diff-filter=%s"), | ||||||
| 				     optarg[i], optarg); | 				     optarg[i], optarg); | ||||||
| 		if (negate) | 		if (negate) | ||||||
| 			opt->filter &= ~bit; | 			opt->filter_not |= bit; | ||||||
| 		else | 		else | ||||||
| 			opt->filter |= bit; | 			opt->filter |= bit; | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
							
								
								
									
										2
									
								
								diff.h
								
								
								
								
							
							
						
						
									
										2
									
								
								diff.h
								
								
								
								
							|  | @ -283,7 +283,7 @@ struct diff_options { | ||||||
| 	struct diff_flags flags; | 	struct diff_flags flags; | ||||||
|  |  | ||||||
| 	/* diff-filter bits */ | 	/* diff-filter bits */ | ||||||
| 	unsigned int filter; | 	unsigned int filter, filter_not; | ||||||
|  |  | ||||||
| 	int use_color; | 	int use_color; | ||||||
|  |  | ||||||
|  |  | ||||||
|  | @ -142,6 +142,19 @@ test_expect_success 'diff-filter=R' ' | ||||||
|  |  | ||||||
| ' | ' | ||||||
|  |  | ||||||
|  | test_expect_success 'multiple --diff-filter bits' ' | ||||||
|  |  | ||||||
|  | 	git log -M --pretty="format:%s" --diff-filter=R HEAD >expect && | ||||||
|  | 	git log -M --pretty="format:%s" --diff-filter=Ra HEAD >actual && | ||||||
|  | 	test_cmp expect actual && | ||||||
|  | 	git log -M --pretty="format:%s" --diff-filter=aR HEAD >actual && | ||||||
|  | 	test_cmp expect actual && | ||||||
|  | 	git log -M --pretty="format:%s" \ | ||||||
|  | 		--diff-filter=a --diff-filter=R HEAD >actual && | ||||||
|  | 	test_cmp expect actual | ||||||
|  |  | ||||||
|  | ' | ||||||
|  |  | ||||||
| test_expect_success 'diff-filter=C' ' | test_expect_success 'diff-filter=C' ' | ||||||
|  |  | ||||||
| 	git log -C -C --pretty="format:%s" --diff-filter=C HEAD >actual && | 	git log -C -C --pretty="format:%s" --diff-filter=C HEAD >actual && | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Johannes Schindelin
						Johannes Schindelin