Re: [PATCH] builtin/stash: configs keepIndex, includeUntracked

From: Ricardo C <rpc01234@gmail.com>
Date: Feb 19, 2024, 21:41 UTC
Message-ID: <cdc377ea-d810-49c6-9124-c450aff34118@gmail.com>
In-Reply-To: <fc7a8c46-61e4-4b5d-b625-cbc845b81590@gmail.com>

Hello,

On 2/19/24 03:04, Jean-Noël Avila wrote:

> I'm not sure this would be better, but instead of mixing option compatibility 
> and actually building your logic, why not use a series of
> die_for_incompatible_opt3 and the like in order to clear the option lists, 
> then build your action logic without resorting to special values?

I'm not sure dying would be appropriate here, since the original behavior was already to clean up buffers and such and then return an errorful value (-1), and I'd rather keep maximum backwards compatibility. Also, the special value of -1 for `keep_index` and `include_untracked` is necessary to detect whether the variable was set by the CLI flags or if it is fine to override with the config value.

This could be addressed by moving more logic up to `push_stash` and `save_stash` (where the arguments are parsed), but this would need much more rewriting and would lead to some code duplication, for what I think is minimal gain.

Thank you,

Ricardo