Re: [PATCH v5 4/6] worktree: factor out dwim_branch function

[Eric Sunshine]

On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer  wrote:
> Factor out a dwim_branch function, which takes care of the dwim'ery in
> 'git worktree add '.  It's not too much code currently, but we're
> adding a new kind of dwim in a subsequent patch, at which point it makes
> more sense to have it as a separate function.
> [...]
> Signed-off-by: Thomas Gummerer 
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -417,16 +431,9 @@ static int add(int ac, const char **av, const char 
> *prefix)
> if (ac < 2 && !opts.new_branch && !opts.detach) {
> -   int n;
> -   const char *s = worktree_basename(path, );
> -   opts.new_branch = xstrndup(s, n);
> -   if (guess_remote) {
> -   struct object_id oid;
> -   const char *remote =
> -   unique_tracking_name(opts.new_branch, );
> -   if (remote)
> -   branch = remote;
> -   }
> +   const char *dwim_branchname = dwim_branch(path, );
> +   if (dwim_branchname)
> +   branch = dwim_branchname;

I don't care strongly but the name 'dwim_branchname' is awfully long
in a context where it's assigned the result of a call to
dwim_branch(). In this tiny code block, a short and sweet name 's'
would be easy to grok; there'd be no confusion.

Anyhow, it's subjective and not at all worth a re-roll.

[PATCH v5 4/6] worktree: factor out dwim_branch function

[Thomas Gummerer]

Factor out a dwim_branch function, which takes care of the dwim'ery in
'git worktree add '.  It's not too much code currently, but we're
adding a new kind of dwim in a subsequent patch, at which point it makes
more sense to have it as a separate function.

Factor it out now to reduce the patch noise in the next patch.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 1e4a919a00..c296c3eacb 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -366,6 +366,20 @@ static int add_worktree(const char *path, const char 
*refname,
return ret;
 }
 
+static const char *dwim_branch(const char *path, struct add_opts *opts)
+{
+   int n;
+   const char *s = worktree_basename(path, );
+   opts->new_branch = xstrndup(s, n);
+   if (guess_remote) {
+   struct object_id oid;
+   const char *remote =
+   unique_tracking_name(opts->new_branch, );
+   return remote;
+   }
+   return NULL;
+}
+
 static int add(int ac, const char **av, const char *prefix)
 {
struct add_opts opts;
@@ -417,16 +431,9 @@ static int add(int ac, const char **av, const char *prefix)
}
 
if (ac < 2 && !opts.new_branch && !opts.detach) {
-   int n;
-   const char *s = worktree_basename(path, );
-   opts.new_branch = xstrndup(s, n);
-   if (guess_remote) {
-   struct object_id oid;
-   const char *remote =
-   unique_tracking_name(opts.new_branch, );
-   if (remote)
-   branch = remote;
-   }
+   const char *dwim_branchname = dwim_branch(path, );
+   if (dwim_branchname)
+   branch = dwim_branchname;
}
 
if (ac == 2 && !opts.new_branch && !opts.detach) {
-- 
2.16.1.77.g8685934aa2