Re: [PATCH v2 1/3] branch: fix die_if_checked_out() when ignore_current_worktree

From: Rubén Justo <rjusto@gmail.com>
Date: Jan 26, 2023, 03:07 UTC
Message-ID: <e7943994-3a7a-f877-8cf2-71146def9076@gmail.com>
In-Reply-To: <1a7fa327-3833-8da3-46d7-60bfe8dae82c@dunelm.org.uk>

On 24-ene-2023 10:35:26, Phillip Wood wrote:

Show 36 quoted lines

> On 22/01/2023 23:21, Rubén Justo wrote:
> > I tried to maintain the relationship and the role, too.  Just introduce
> > the helper, as Phillip suggested and I think it is a good idea.
> 
> When I suggested adding a helper I was thinking of something like
> 
> static const struct worktree *do_find_shared_symref(struct worktree
> **worktrees,
>  					  const char *symref,
>  					  const char *target,
> 					  int ignore_current)
> {
> 	/*
> 	 * Body moved from find_share_symref() with a couple
> 	 * of lines added to support ignore_current
> 	 /*
> }
> 
> const struct worktree *find_shared_symref(struct worktree **worktrees,
>  					  const char *symref,
>  					  const char *target)
> {
> 	return do_find_shared_symref(worktrees, symref, target, 0)
> }
> 
> void die_if_checked_out(const char *branch, int ignore_current_worktree)
> {
> 	struct worktree **worktrees = get_worktrees();
> 	const struct worktree *wt;
> 
> 	wt = do_find_shared_symref(worktrees, "HEAD", branch,
> 				   ignore_current_worktree);
> 	/* rest unchanged */
> }
> 
> The aim was to avoid changing the public api

I thought about a solution like the one you suggest. Also another one based on iterations, something like wt_head_refs().... I ended up with is_shared_symref(), it adds some value, I think.

The public API remains unchanged, and I like that the comment for find_shared_ref(), which is an important note, is moved to is_shared_symref(), which contains the essential work related to the comment.

die_if_checked_out() needs to iterate (here was the wt_heads_refs()), but my intuition makes me think it's a good step since we might need another level, I'm not sure yet but, like "die_if_if_checked_out(allow_if_current)".

I'm going to send a v3 addressing the issues Junio commented, still with the is_shared_symref(). If the above reasoning doesn't work for you or if the change as-is introduces any problem, I have no objection to "do_find_shared_symref()".

Thank you.