Re: [PATCH v3 3/8] [RFC] ls-files: error out on -i unless -o or -c are specified

[Junio C Hamano <gitster@pobox.com>]

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> @@ -748,6 +748,9 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>  	if (pathspec.nr && error_unmatch)
>  		ps_matched = xcalloc(pathspec.nr, 1);
>  
> +	if ((dir.flags & DIR_SHOW_IGNORED) && !show_others && !show_cached)
> +		die("ls-files --ignored is usually used with --others, but --cached is the default.  Please specify which you want.");
> +

So "git ls-files -i" would suddenly start erroring out and users are
to scramble and patch their scripts?

More importantly, the message does not make much sense.  "I is
usually used with O" is very true, but the mention of "usually" here
means it is not an error for "I" to be used without "O".  That part
is very understandable and correct.

But I do not know what "but --cached is the default" part wants to
say.  If it is the _default_, and (assuming that what I read in the
proposed log message is correct) the combination of "-i -c" is valid,
then I would understand the message if the code were more like this:

	if ((dir.flags & DIR_SHOW_IGNORED) &&
	    !show_others && !show_cached) {
		show_cached = 1; /* default */
		warning("ls-files -i given without -o/-c; defaulting to -i -c");
	}

If we are not defaulting to cached, then

	die("ls-files -i must be used with either -o or -c");

would also make sense.

The variant presented in the patch does not make sense to me.

> diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
> index dd87b43be1a6..40d3c42618c0 100755
> --- a/t/t1306-xdg-files.sh
> +++ b/t/t1306-xdg-files.sh
> @@ -116,7 +116,7 @@ test_expect_success 'Exclusion in a non-XDG global ignore file' '
>  test_expect_success 'Checking XDG ignore file when HOME is unset' '
>  	(sane_unset HOME &&
>  	 git config --unset core.excludesfile &&
> -	 git ls-files --exclude-standard --ignored >actual) &&
> +	 git ls-files --exclude-standard --ignored --others >actual) &&
>  	test_must_be_empty actual
>  '
>  
> diff --git a/t/t3003-ls-files-exclude.sh b/t/t3003-ls-files-exclude.sh
> index d5ec333131f9..c41c4f046abf 100755
> --- a/t/t3003-ls-files-exclude.sh
> +++ b/t/t3003-ls-files-exclude.sh
> @@ -29,11 +29,11 @@ test_expect_success 'add file to gitignore' '
>  '
>  check_all_output
>  
> -test_expect_success 'ls-files -i lists only tracked-but-ignored files' '
> +test_expect_success 'ls-files -i -c lists only tracked-but-ignored files' '
>  	echo content >other-file &&
>  	git add other-file &&
>  	echo file >expect &&
> -	git ls-files -i --exclude-standard >output &&
> +	git ls-files -i -c --exclude-standard >output &&
>  	test_cmp expect output
>  '