Re: [PATCH v5 3/6] worktree: remove force_new_branch from struct add_opts

2018-03-30 Thread Thomas Gummerer
On 03/27, Eric Sunshine wrote:
> On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer  wrote:
> > The 'force_new_branch' flag in 'struct add_opts' is only used inside the
> > add function, where we already have the same information stored in the
> > 'new_branch_force' variable.  Avoid that unnecessary duplication.
> 
> When I was reviewing your original dwim-ery series (inferring 'foo'
> from 'origin/foo'), I noticed that 'struct add_opts' had accumulated a
> number of unnecessary members over time, including this one. My plan
> was to submit a patch removing all those pointless members after your
> dwim-ery series settled, however, I never got around to it.

Ah right, I didn't look at them in detail, so I failed to notice that.
While I'm already in the area I may as well do that, thanks for the
suggestion!

> This patch might be a good opportunity for doing that cleanup
> wholesale, removing all unneeded members rather than just the one. (If
> so, you'd probably want to move to this patch to the front of the
> series as cleanup before the meatier changes.) Anyhow, it's just a
> thought; feel free to respond with "it's outside the scope of this
> series" if you're not interested in doing it.
> 
> > Signed-off-by: Thomas Gummerer 


Re: [PATCH v5 3/6] worktree: remove force_new_branch from struct add_opts

2018-03-27 Thread Eric Sunshine
On Sun, Mar 25, 2018 at 9:49 AM, Thomas Gummerer  wrote:
> The 'force_new_branch' flag in 'struct add_opts' is only used inside the
> add function, where we already have the same information stored in the
> 'new_branch_force' variable.  Avoid that unnecessary duplication.

When I was reviewing your original dwim-ery series (inferring 'foo'
from 'origin/foo'), I noticed that 'struct add_opts' had accumulated a
number of unnecessary members over time, including this one. My plan
was to submit a patch removing all those pointless members after your
dwim-ery series settled, however, I never got around to it.

This patch might be a good opportunity for doing that cleanup
wholesale, removing all unneeded members rather than just the one. (If
so, you'd probably want to move to this patch to the front of the
series as cleanup before the meatier changes.) Anyhow, it's just a
thought; feel free to respond with "it's outside the scope of this
series" if you're not interested in doing it.

> Signed-off-by: Thomas Gummerer 


[PATCH v5 3/6] worktree: remove force_new_branch from struct add_opts

2018-03-25 Thread Thomas Gummerer
The 'force_new_branch' flag in 'struct add_opts' is only used inside the
add function, where we already have the same information stored in the
'new_branch_force' variable.  Avoid that unnecessary duplication.

Signed-off-by: Thomas Gummerer 
---
 builtin/worktree.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index a082230b6c..1e4a919a00 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -28,7 +28,6 @@ struct add_opts {
int checkout;
int keep_locked;
const char *new_branch;
-   int force_new_branch;
 };
 
 static int show_only;
@@ -405,8 +404,7 @@ static int add(int ac, const char **av, const char *prefix)
if (!strcmp(branch, "-"))
branch = "@{-1}";
 
-   opts.force_new_branch = !!new_branch_force;
-   if (opts.force_new_branch) {
+   if (new_branch_force) {
struct strbuf symref = STRBUF_INIT;
 
opts.new_branch = new_branch_force;
@@ -450,7 +448,7 @@ static int add(int ac, const char **av, const char *prefix)
struct child_process cp = CHILD_PROCESS_INIT;
cp.git_cmd = 1;
argv_array_push(&cp.args, "branch");
-   if (opts.force_new_branch)
+   if (new_branch_force)
argv_array_push(&cp.args, "--force");
argv_array_push(&cp.args, opts.new_branch);
argv_array_push(&cp.args, branch);
-- 
2.16.1.77.g8685934aa2