Re: [PATCH v2] stash: honor --no-overwrite-ignore with --all

[Pushkar Singh <pushkarkumarsingh1970@gmail.com>]

Hi Elijah,

Thanks for the detailed feedback. Much appreciated.

> What's the basis for this patch? I don't see any "overwrite-ignore"
> anywhere in builtin/stash.c .

The basis was the existing behavior where git stash push -a would remove
ignored files even when --no-overwrite-ignore is provided. The intent was
to make stash honor --no-overwrite-ignore consistently with other callers
of unpack_trees, limited specifically to the stash -a cleanup path.

In v3 I rebased onto current master and also removed the commit message
claim about removing the stash FIXME, since this series only addresses
the concrete stash behavior and does not attempt to solve the broader
unpack_trees issues.

> This suggests that --all and --no-overwrite-ignore are incompatible,
> yes? Shouldn't they be reported as such rather than having one silently
> override the other?

I agree they are philosophically contradictory. I chose to downgrade
INCLUDE_ALL_FILES to include-untracked when --no-overwrite-ignore is given
so that users explicitly requesting preservation of ignored files are not
surprised by their removal.

I am open to changing this to an explicit error instead if that is
preferred. I went with downgrading to preserve backwards compatibility
and to honor the more conservative option.

> Also, the commit message claims you are removing a FIXME comment, but
> no such removal is found in this patch.

Yes, that was an error in v2. In v3 the commit message no longer claims
to remove the FIXME and is scoped only to fixing stash -a behavior plus
adding regression tests.

Regarding tests, v3 now explicitly covers both:

- stash push -a --no-overwrite-ignore preserving ignored files
- stash push -a --overwrite-ignore removing them

using test_grep and test_path_is_missing as suggested.

Thanks also for the references to the broader unpack_trees and stash
history. I understand this patch only addresses a small part of a much
larger and messier area.

Please let me know if you would prefer the -a plus --no-overwrite-ignore
combination to error out instead of downgrading.

Thanks,
Pushkar