Re: [PATCH v7 2/3] worktree: move/refactor find_shared_symref from branch.c
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 11, 2015, 16:16 UTC
- Message-ID
- <xmqqk2rx0w54.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1441402769-35897-3-git-send-email-rappazzo@gmail.com>
Michael Rappazzo <rappazzo@gmail.com> writes:
Show 7 quoted lines
> The code formerly in branch.c was largely the basis of the > get_worktree_list implementation is now moved to worktree.c, > and the find_shared_symref implementation has been refactored > to use get_worktree_list > > Signed-off-by: Michael Rappazzo <rappazzo@gmail.com> > ---
Show 25 quoted lines
> branch.c | 79 +--------------------------------------------------------
> branch.h | 8 ------
> builtin/notes.c | 1 +
> worktree.c | 40 +++++++++++++++++++++++++++++
> worktree.h | 7 +++++
> 5 files changed, 49 insertions(+), 86 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index d013374..77d7f2a 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -4,6 +4,7 @@
> #include "refs.h"
> #include "remote.h"
> #include "commit.h"
> +#include "worktree.h"
>
> struct tracking {
> struct refspec spec;
> @@ -311,84 +312,6 @@ void remove_branch_state(void)
> unlink(git_path_squash_msg());
> }
>
> -static char *find_linked_symref(const char *symref, const char *branch,
> - const char *id)The title refers to a function with a different name ;-).
Copying the bulk of the function in 1/3 and then removing the original here made it somewhat hard to compare what got changed in the implementation.
I _think_ the code structure in the end result is more or less right, though.
Show 26 quoted lines
> diff --git a/worktree.c b/worktree.c
> index 33d2e57..e45b651 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -155,3 +155,43 @@ done:
> return list;
> }
>
> +char *find_shared_symref(const char *symref, const char *target)
> +{
> + char *existing = NULL;
> + struct strbuf path = STRBUF_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + struct worktree_list *worktree_list = get_worktree_list();
> + struct worktree_list *orig_list = worktree_list;
> + struct worktree *matched = NULL;
> +
> + while (!matched && worktree_list) {
> + if (strcmp("HEAD", symref)) {
> + strbuf_reset(&path);
> + strbuf_reset(&sb);
> + strbuf_addf(&path, "%s/%s", worktree_list->worktree->git_dir, symref);
> +
> + if (_parse_ref(path.buf, &sb, NULL)) {
> + continue;
> + }I forgot to say this on 1/3, but this helper function _parse_ref() is obviously an implementation detail; does it have to be visible outside the file, or can it become file-scope static? There may be other helpers that may not want to be global (I didn't check).
Provided that 1/3 will become accepable, this step looks sensible to me.
Thanks.