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()) | ||||
| 		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); | ||||
| } | ||||
|  | ||||
|  | @ -4820,21 +4826,6 @@ static int diff_opt_diff_filter(const struct option *option, | |||
| 	BUG_ON_OPT_NEG(unset); | ||||
| 	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; | ||||
|  | @ -4851,7 +4842,7 @@ static int diff_opt_diff_filter(const struct option *option, | |||
| 			return error(_("unknown change class '%c' in --diff-filter=%s"), | ||||
| 				     optarg[i], optarg); | ||||
| 		if (negate) | ||||
| 			opt->filter &= ~bit; | ||||
| 			opt->filter_not |= bit; | ||||
| 		else | ||||
| 			opt->filter |= bit; | ||||
| 	} | ||||
|  |  | |||
							
								
								
									
										2
									
								
								diff.h
								
								
								
								
							
							
						
						
									
										2
									
								
								diff.h
								
								
								
								
							|  | @ -283,7 +283,7 @@ struct diff_options { | |||
| 	struct diff_flags flags; | ||||
|  | ||||
| 	/* diff-filter bits */ | ||||
| 	unsigned int filter; | ||||
| 	unsigned int filter, filter_not; | ||||
|  | ||||
| 	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' ' | ||||
|  | ||||
| 	git log -C -C --pretty="format:%s" --diff-filter=C HEAD >actual && | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 Johannes Schindelin
						Johannes Schindelin