diff mbox series

[03/15] push: reorder switch cases

Message ID 20210529074458.1916817-4-felipe.contreras@gmail.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Felipe Contreras May 29, 2021, 7:44 a.m. UTC
We want all the cases that don't do anything with a branch first, and
then the rest.

Will help further patches.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/push.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Junio C Hamano May 31, 2021, 6:34 a.m. UTC | #1
Felipe Contreras <felipe.contreras@gmail.com> writes:

> We want all the cases that don't do anything with a branch first, and
> then the rest.
>
> Will help further patches.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/push.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index f4e919450d..c19321bb9d 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -254,11 +254,19 @@ static void setup_default_push_refspecs(struct remote *remote)
>  	int triangular = is_workflow_triangular(remote);
>  
>  	switch (push_default) {
> -	default:
>  	case PUSH_DEFAULT_MATCHING:
>  		refspec_append(&rs, ":");
>  		return;
>  
> +	case PUSH_DEFAULT_NOTHING:
> +		die(_("You didn't specify any refspecs to push, and "
> +		    "push.default is \"nothing\"."));
> +		return;
> +	default:
> +	}
> +
> +	switch (push_default) {
> +	default:

This is not quite "reorder" but split into two.  It is not yet clear
how it helps, but hopefully we'll find out why splitting the switch
into two switches is a good idea soon in a later step, so the title
needs updating to sell that aspect of the change (unless it is a
pointless change made by mistake that the originally-single switch
got split into two, but that is unlikely the case ;-).

>  	case PUSH_DEFAULT_UNSPECIFIED:
>  	case PUSH_DEFAULT_SIMPLE:
>  		setup_push_simple(remote, branch, triangular);
> @@ -271,11 +279,6 @@ static void setup_default_push_refspecs(struct remote *remote)
>  	case PUSH_DEFAULT_CURRENT:
>  		setup_push_current(remote, branch);
>  		return;
> -
> -	case PUSH_DEFAULT_NOTHING:
> -		die(_("You didn't specify any refspecs to push, and "
> -		    "push.default is \"nothing\"."));
> -		return;
>  	}
>  }
Felipe Contreras May 31, 2021, 8:14 a.m. UTC | #2
Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > We want all the cases that don't do anything with a branch first, and
> > then the rest.
> >
> > Will help further patches.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  builtin/push.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/builtin/push.c b/builtin/push.c
> > index f4e919450d..c19321bb9d 100644
> > --- a/builtin/push.c
> > +++ b/builtin/push.c
> > @@ -254,11 +254,19 @@ static void setup_default_push_refspecs(struct remote *remote)
> >  	int triangular = is_workflow_triangular(remote);
> >  
> >  	switch (push_default) {
> > -	default:
> >  	case PUSH_DEFAULT_MATCHING:
> >  		refspec_append(&rs, ":");
> >  		return;
> >  
> > +	case PUSH_DEFAULT_NOTHING:
> > +		die(_("You didn't specify any refspecs to push, and "
> > +		    "push.default is \"nothing\"."));
> > +		return;
> > +	default:
> > +	}
> > +
> > +	switch (push_default) {
> > +	default:
> 
> This is not quite "reorder" but split into two.

OK.

> It is not yet clear
> how it helps, but hopefully we'll find out why splitting the switch
> into two switches is a good idea soon in a later step, so the title
> needs updating to sell that aspect of the change

How about: By splitting the two kinds of modes we can return sooner in
the case the mode doesn't need to do anything else, and for the rest
that reuse quite a bit of code it can be set up after the first switch.
diff mbox series

Patch

diff --git a/builtin/push.c b/builtin/push.c
index f4e919450d..c19321bb9d 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -254,11 +254,19 @@  static void setup_default_push_refspecs(struct remote *remote)
 	int triangular = is_workflow_triangular(remote);
 
 	switch (push_default) {
-	default:
 	case PUSH_DEFAULT_MATCHING:
 		refspec_append(&rs, ":");
 		return;
 
+	case PUSH_DEFAULT_NOTHING:
+		die(_("You didn't specify any refspecs to push, and "
+		    "push.default is \"nothing\"."));
+		return;
+	default:
+	}
+
+	switch (push_default) {
+	default:
 	case PUSH_DEFAULT_UNSPECIFIED:
 	case PUSH_DEFAULT_SIMPLE:
 		setup_push_simple(remote, branch, triangular);
@@ -271,11 +279,6 @@  static void setup_default_push_refspecs(struct remote *remote)
 	case PUSH_DEFAULT_CURRENT:
 		setup_push_current(remote, branch);
 		return;
-
-	case PUSH_DEFAULT_NOTHING:
-		die(_("You didn't specify any refspecs to push, and "
-		    "push.default is \"nothing\"."));
-		return;
 	}
 }