Re: [PATCH v3] stash: implement '--staged' option for 'push' and 'save'

[Sergey Organov <sorganov@gmail.com>]

Jeff King <peff@peff.net> writes:

> On Mon, Oct 18, 2021 at 07:09:06PM +0300, Sergey Organov wrote:
>
>> +static int stash_staged(struct stash_info *info, const struct pathspec *ps,
>> +			struct strbuf *out_patch, int quiet)
>> +{
>> +	int ret = 0;
>> +	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
>> +	struct index_state istate = { NULL };
>> +
>> + if (write_index_as_tree(&info->w_tree, &istate,
>> the_repository->index_file,
>> +				0, NULL)) {
>> +		ret = -1;
>> +		goto done;
>> +	}
>> +
>> +	cp_diff_tree.git_cmd = 1;
>> +	strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD",
>> +		     oid_to_hex(&info->w_tree), "--", NULL);
>> +	if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) {
>> +		ret = -1;
>> +		goto done;
>> +	}
>> +
>> +	if (!out_patch->len) {
>> +		if (!quiet)
>> +			fprintf_ln(stderr, _("No staged changes"));
>> +		ret = 1;
>> +	}
>> +
>> +done:
>> +	discard_index(&istate);
>> +	return ret;
>> +}
>
> This function doesn't look at its "struct pathspec" parameter at all.
> I'm not sure if that's a bug (i.e., it should be restricting the diff
> here) or if it was just pulled unnecessarily over from stash_patch().

Yep, it's a remnant from copy-paste of stash_patch(). I'm used to
getting a warning from compiler for such cases and didn't pay enough
attention. The warning belongs to -Wextra though and is not turned on
for Git compilation.

Thanks for catching!

-- Sergey Organov