Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:10

Ralf Thielow [off-list ref] writes:
Show the usage with options for 'checkout' command on missing argument 'branch'.
Please describe what exact command line you typed, what output and side
effect you got from the command, what you _think_ should have happened
instead, and what the differences between two are.

In other words, what problem you are trying to solve?
quoted hunk
Signed-off-by: Ralf Thielow <redacted>
---
 builtin/checkout.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 38632fc..4aa613a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1086,5 +1086,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (opts.writeout_stage)
 		die(_("--ours/--theirs is incompatible with switching branches."));
 
+	if (new.name == NULL) {
+		usage_with_options(checkout_usage, options);
+		return;
+	}

 	return switch_branches(&opts, &new);
 }
What value are you returning from a function whose return type is int?

If you read the function switch_branches(), you would notice that it is
prepared to handle the case where new.name is NULL (by the way, check
against NULL is typically spelled as "if (!new.name)" as you can see
there), and then would realize that your change by itself cannot be a
correct fix for whatever problem you are trying to solve.

Have you run "make test" at all?

If you are changing this command:

	$ git checkout -b frotz
	Switched to a new branch 'junk'

to error out with a message, then that is a regression.

Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'

From: Ralf Thielow <hidden>
Date: 2016-06-15 22:51:10

Simple "git checkout" or "git checkout " don't tell me that i've done a mistake
on usage. It does nothing. This can be different when using parameters, but in
the end i think it should tell me the usage in this case.

2011/5/5 Junio C Hamano [off-list ref]:
Ralf Thielow [off-list ref] writes:
quoted
Show the usage with options for 'checkout' command on missing argument 'branch'.
Please describe what exact command line you typed, what output and side
effect you got from the command, what you _think_ should have happened
instead, and what the differences between two are.

In other words, what problem you are trying to solve?
quoted
Signed-off-by: Ralf Thielow <redacted>
---
 builtin/checkout.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 38632fc..4aa613a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1086,5 +1086,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
      if (opts.writeout_stage)
              die(_("--ours/--theirs is incompatible with switching branches."));

+     if (new.name == NULL) {
+             usage_with_options(checkout_usage, options);
+             return;
+     }

      return switch_branches(&opts, &new);
 }
What value are you returning from a function whose return type is int?

If you read the function switch_branches(), you would notice that it is
prepared to handle the case where new.name is NULL (by the way, check
against NULL is typically spelled as "if (!new.name)" as you can see
there), and then would realize that your change by itself cannot be a
correct fix for whatever problem you are trying to solve.

Have you run "make test" at all?

If you are changing this command:

       $ git checkout -b frotz
       Switched to a new branch 'junk'

to error out with a message, then that is a regression.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help