git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Han-Wen Nienhuys <hanwenn@gmail.com>,
	Han-Wen Nienhuys <hanwen@google.com>
Subject: Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()
Date: Tue, 13 Sep 2022 08:43:07 -0700	[thread overview]
Message-ID: <xmqqillrb7qs.fsf@gitster.g> (raw)
In-Reply-To: <xmqq35cwcpws.fsf@gitster.g> (Junio C. Hamano's message of "Mon, 12 Sep 2022 13:13:07 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Otherwise we fall through with worktree_ref that we have stripped
>> main-worktree/ prefix, which means the original input
>>
>> 	main-worktree/worktrees/foo/blah
>>
>> is now 
>>
>> 	worktrees/foo/blah
>>
>> and the next skip_prefix() will see that it begins with "worktrees/".
>> Of course, if the initial input were
>>
>> 	worktrees/foo/blah
>>
>> then we wouldn't have skipped main-worktree/ prefix from it, and go
>> to the next skip_prefix().  So from here on, we cannot tell which
>> case the original input was.
>>
>> But that is OK.  Asking "give me the ref 'blah' in the worktree 'foo'"
>> in the current worktree should yield the same answer to the question
>> "give me the ref 'blah' in the worktree 'foo', as if I asked you to
>> do so in the main worktree".
>
> This makes me wonder...
>
> I wonder if it makes the resulting code clearer to go fully
> recursive, unlike the posted code that says "if a recursive call
> says it is for current, that means it is for main worktree, and
> otherwise pretend as if the input did not have the prefix".
>
> That is, something like
> ...

The above may be a wrong suggestion, as it was solely guided by my
reading of the posted code that looked as if it wanted to support
something like "main-worktree/worktrees/foo/refs/heads/main".  If
that wasn't the intention, and we only want to support

  1. a ref spelled in the traditional way before multiple worktree feature,
  2. 1., with "main-worktree/" prefixed, or
  3. 1., with "worktrees/<worktreename>/" prefixed

then a better approach would be to have a small helper
parse_local_worktree_ref() and make the primary one into something
like

parse_worktree_ref()
{
        if (begins with "main-worktree/") {	
		strip main-worktree/ prefix;
		switch (parse_local_worktree_ref(input minus prefix)) {
		... the same switch to turn "ours" into "main's" ...
		... but we probably want to special case if we are ...
		... the main worktree ...
		}
	}
	else if (begins with "worktrees/") {
		strip worktrees/ prefix and learn worktree name;
		switch (parse_local_worktree_ref(input minus prefix)) {
		... the same switch to turn "ours" into "theirs" ...
		... but we probably want to special case if we are ...
		... the worktree in question ...
		}
	}
	return parse_local_worktree_ref(input);
}

and have parse_local_worktree_ref() handle psuedorefs,
per-worktree-refs.

  reply	other threads:[~2022-09-13 16:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-12 17:01 [PATCH] refs: unify parse_worktree_ref() and ref_type() Han-Wen Nienhuys via GitGitGadget
2022-09-12 19:17 ` Junio C Hamano
2022-09-12 20:13   ` Junio C Hamano
2022-09-13 15:43     ` Junio C Hamano [this message]
2022-09-19 14:28       ` Han-Wen Nienhuys
2022-09-19 21:43         ` Junio C Hamano
2022-09-20  8:53           ` Han-Wen Nienhuys
2022-09-21 16:45             ` Junio C Hamano
2022-09-19 16:34 ` [PATCH v2] " Han-Wen Nienhuys via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqillrb7qs.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).