Re: [PATCH v4 04/10] reset: modernize flags passed to `reset_working_tree()`

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Patrick

On 10/06/2026 09:52, Patrick Steinhardt wrote:
> The flags passed to `reset_working_tree()` are declared as defines. This
> has fallen a bit out of practice nowadays, where we instead prefer to
> use enums. Furthermore, the prefix of those flags does not match the
> function name anymore after the rename in the preceding commit.
> 
> Adapt the code to follow modern best practices and adapt the flag names.

This looks good

Thanks

Phillip

> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   builtin/rebase.c | 15 ++++++++-------
>   reset.c          | 12 ++++++------
>   reset.h          | 31 +++++++++++++++++++------------
>   sequencer.c      |  9 ++++++---
>   4 files changed, 39 insertions(+), 28 deletions(-)
> 
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 22fbba3c62..06dcbaf5e8 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -607,7 +607,7 @@ static int move_to_original_branch(struct rebase_options *opts)
>   	strbuf_addf(&head_reflog, "%s (finish): returning to %s",
>   		    opts->reflog_action, opts->head_name);
>   	ropts.branch = opts->head_name;
> -	ropts.flags = RESET_HEAD_REFS_ONLY;
> +	ropts.flags = RESET_WORKING_TREE_REFS_ONLY;
>   	ropts.branch_msg = branch_reflog.buf;
>   	ropts.head_msg = head_reflog.buf;
>   	ret = reset_working_tree(the_repository, &ropts);
> @@ -862,9 +862,9 @@ static int checkout_up_to_date(struct rebase_options *options)
>   		    options->reflog_action, options->switch_to);
>   	ropts.oid = &options->orig_head->object.oid;
>   	ropts.branch = options->head_name;
> -	ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
> +	ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
>   	if (!ropts.branch)
> -		ropts.flags |=  RESET_HEAD_DETACH;
> +		ropts.flags |=  RESET_WORKING_TREE_DETACH;
>   	ropts.head_msg = buf.buf;
>   	if (reset_working_tree(the_repository, &ropts) < 0)
>   		ret = error(_("could not switch to %s"), options->switch_to);
> @@ -1384,7 +1384,7 @@ int cmd_rebase(int argc,
>   
>   		rerere_clear(the_repository, &merge_rr);
>   		string_list_clear(&merge_rr, 1);
> -		ropts.flags = RESET_HEAD_HARD;
> +		ropts.flags = RESET_WORKING_TREE_HARD;
>   		if (reset_working_tree(the_repository, &ropts) < 0)
>   			die(_("could not discard worktree changes"));
>   		remove_branch_state(the_repository, 0);
> @@ -1409,7 +1409,7 @@ int cmd_rebase(int argc,
>   		ropts.oid = &options.orig_head->object.oid;
>   		ropts.head_msg = head_msg.buf;
>   		ropts.branch = options.head_name;
> -		ropts.flags = RESET_HEAD_HARD;
> +		ropts.flags = RESET_WORKING_TREE_HARD;
>   		if (reset_working_tree(the_repository, &ropts) < 0)
>   			die(_("could not move back to %s"),
>   			    oid_to_hex(&options.orig_head->object.oid));
> @@ -1876,8 +1876,9 @@ int cmd_rebase(int argc,
>   		    options.reflog_action, options.onto_name);
>   	ropts.oid = &options.onto->object.oid;
>   	ropts.orig_head = &options.orig_head->object.oid;
> -	ropts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
> -			RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
> +	ropts.flags = RESET_WORKING_TREE_DETACH |
> +		      RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
> +		      RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
>   	ropts.head_msg = msg.buf;
>   	ropts.default_reflog_action = options.reflog_action;
>   	if (reset_working_tree(the_repository, &ropts)) {
> diff --git a/reset.c b/reset.c
> index 799596398b..4ca7f23a25 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -16,9 +16,9 @@ static int update_refs(struct repository *repo,
>   		       const struct object_id *oid,
>   		       const struct object_id *head)
>   {
> -	unsigned detach_head = opts->flags & RESET_HEAD_DETACH;
> -	unsigned run_hook = opts->flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
> -	unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
> +	unsigned detach_head = opts->flags & RESET_WORKING_TREE_DETACH;
> +	unsigned run_hook = opts->flags & RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
> +	unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
>   	const struct object_id *orig_head = opts->orig_head;
>   	const char *switch_to_branch = opts->branch;
>   	const char *reflog_branch = opts->branch_msg;
> @@ -90,9 +90,9 @@ int reset_working_tree(struct repository *r,
>   {
>   	const struct object_id *oid = opts->oid;
>   	const char *switch_to_branch = opts->branch;
> -	unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
> -	unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
> -	unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
> +	unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
> +	unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
> +	unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
>   	struct object_id *head = NULL, head_oid;
>   	struct tree_desc desc[2] = { { NULL }, { NULL } };
>   	struct lock_file lock = LOCK_INIT;
> diff --git a/reset.h b/reset.h
> index f130152014..2e5826de99 100644
> --- a/reset.h
> +++ b/reset.h
> @@ -6,16 +6,22 @@
>   
>   #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
>   
> -/* Request a detached checkout */
> -#define RESET_HEAD_DETACH (1<<0)
> -/* Request a reset rather than a checkout */
> -#define RESET_HEAD_HARD (1<<1)
> -/* Run the post-checkout hook */
> -#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
> -/* Only update refs, do not touch the worktree */
> -#define RESET_HEAD_REFS_ONLY (1<<3)
> -/* Update ORIG_HEAD as well as HEAD */
> -#define RESET_ORIG_HEAD (1<<4)
> +enum reset_working_tree_flags {
> +	/* Request a detached checkout */
> +	RESET_WORKING_TREE_DETACH = (1 << 0),
> +
> +	/* Request a reset rather than a checkout */
> +	RESET_WORKING_TREE_HARD = (1 << 1),
> +
> +	/* Run the post-checkout hook */
> +	RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK = (1 << 2),
> +
> +	/* Only update refs, do not touch the worktree */
> +	RESET_WORKING_TREE_REFS_ONLY = (1 << 3),
> +
> +	/* Update ORIG_HEAD as well as HEAD */
> +	RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
> +};
>   
>   struct reset_working_tree_options {
>   	/*
> @@ -33,7 +39,7 @@ struct reset_working_tree_options {
>   	/*
>   	 * Flags defined above.
>   	 */
> -	unsigned flags;
> +	enum reset_working_tree_flags flags;
>   	/*
>   	 * Optional reflog message for branch, defaults to head_msg.
>   	 */
> @@ -45,7 +51,8 @@ struct reset_working_tree_options {
>   	const char *head_msg;
>   	/*
>   	 * Optional reflog message for ORIG_HEAD, if this omitted and flags
> -	 * contains RESET_ORIG_HEAD then default_reflog_action must be given.
> +	 * contains RESET_WORKING_TREE_UPDATE_ORIG_HEAD then
> +	 * default_reflog_action must be given.
>   	 */
>   	const char *orig_head_msg;
>   	/*
> diff --git a/sequencer.c b/sequencer.c
> index d73ecf0384..4efe831178 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4677,7 +4677,9 @@ static void create_autostash_internal(struct repository *r,
>   	if (has_unstaged_changes(r, 1) ||
>   	    has_uncommitted_changes(r, 1)) {
>   		struct child_process stash = CHILD_PROCESS_INIT;
> -		struct reset_working_tree_options ropts = { .flags = RESET_HEAD_HARD };
> +		struct reset_working_tree_options ropts = {
> +			.flags = RESET_WORKING_TREE_HARD,
> +		};
>   		struct object_id oid;
>   
>   		strvec_pushl(&stash.args,
> @@ -4870,8 +4872,9 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
>   	struct reset_working_tree_options ropts = {
>   		.oid = onto,
>   		.orig_head = orig_head,
> -		.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
> -				RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
> +		.flags = RESET_WORKING_TREE_DETACH |
> +			 RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
> +			 RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK,
>   		.head_msg = reflog_message(opts, "start", "checkout %s",
>   					   onto_name),
>   		.default_reflog_action = sequencer_reflog_action(opts)
>