| Message ID | 20221205133525.60464-2-tenglong.tl@alibaba-inc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | push: introduce '--heads' option | expand |
Hi, Teng Long <dyroneteng@gmail.com> 于2022年12月5日周一 21:44写道： > > From: Teng Long <dyroneteng@gmail.com> > > The '--all' option of git-push built-in cmd support to push all branches > (refs under refs/heads) to remote. Under the usage, a user can easlily > work in some scenarios, for example, branches synchronization and batch > upload. > > '--all' was introduced for a long time, meanwhile, git supports to > customize the storage location under "refs/". when a new git user see > the usage like, 'git push origin --all', we might feel like we're > pushing _all_ the refs instead of just branches without looking at the > documents until we found the related description of it or '--mirror'. > "--all" sounds like it will include all things: branches, tags, but it only includes branches under ref/heads/, which does cause a little confusion for users. > To ensure compatibility, we cannot rename '--all' to another name > directly, one way is, we can try to add a new option '--heads' which be > identical with the functionality of '--all' to let the user understand > the meaning of representation more clearly. Actually, We've more or less > named options this way already, for example, in 'git-show-ref' and 'git > ls-remote'. > > At the same time, we fix a related issue about the wrong help > information of '--all' option in code. > > Signed-off-by: Teng Long <dyroneteng@gmail.com> > --- > Documentation/git-push.txt | 1 + > builtin/push.c | 13 +++++++------ > t/t5523-push-upstream.sh | 19 ++++++++++++------- > 3 files changed, 20 insertions(+), 13 deletions(-) > > diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt > index 5bb1d5aae25..a5d18fb90b6 100644 > --- a/Documentation/git-push.txt > +++ b/Documentation/git-push.txt > @@ -147,6 +147,7 @@ already exists on the remote side. > `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`. > > --all:: > +--heads:: > Push all branches (i.e. refs under `refs/heads/`); cannot be > used with other <refspec>. > > diff --git a/builtin/push.c b/builtin/push.c > index 60ac8017e52..970cabaa78b 100644 > --- a/builtin/push.c > +++ b/builtin/push.c > @@ -588,11 +588,12 @@ int cmd_push(int argc, const char **argv, const char *prefix) > struct option options[] = { > OPT__VERBOSITY(&verbosity), > OPT_STRING( 0 , "repo", &repo, N_("repository"), N_("repository")), > - OPT_BIT( 0 , "all", &flags, N_("push all refs"), TRANSPORT_PUSH_ALL), > + OPT_BIT( 0 , "all", &flags, N_("push all branches"), TRANSPORT_PUSH_ALL), > + OPT_BIT( 0 , "heads", &flags, N_("push all branches"), TRANSPORT_PUSH_ALL), Maybe OPT_ALIAS() will be better?
> > '--all' was introduced for a long time, meanwhile, git supports to > > customize the storage location under "refs/". when a new git user see > > the usage like, 'git push origin --all', we might feel like we're > > pushing _all_ the refs instead of just branches without looking at the > > documents until we found the related description of it or '--mirror'. > > > > "--all" sounds like it will include all things: branches, tags, but it only > includes branches under ref/heads/, which does cause a little confusion > for users. "under refs/heads", right （^ヮ^）? But The overall is nice to me, I'm sure I will use some to revise. > Maybe OPT_ALIAS() will be better? I think it's suitable for this if there is no misunderstanding for 'OPT_ALIAS()'. Thanks.
Teng Long wrote: > From: Teng Long <dyroneteng@gmail.com> > > The '--all' option of git-push built-in cmd support to push all branches > (refs under refs/heads) to remote. Under the usage, a user can easlily > work in some scenarios, for example, branches synchronization and batch > upload. > > '--all' was introduced for a long time, meanwhile, git supports to > customize the storage location under "refs/". when a new git user see > the usage like, 'git push origin --all', we might feel like we're > pushing _all_ the refs instead of just branches without looking at the > documents until we found the related description of it or '--mirror'. Completely agree. This is something I spotted a long time ago. Although I would prefer `--branches` over `--heads`.
Felipe Contreras <felipe.contreras@gmail.com> writes: > Teng Long wrote: >> From: Teng Long <dyroneteng@gmail.com> >> >> The '--all' option of git-push built-in cmd support to push all branches >> (refs under refs/heads) to remote. Under the usage, a user can easlily >> work in some scenarios, for example, branches synchronization and batch >> upload. >> >> '--all' was introduced for a long time, meanwhile, git supports to >> customize the storage location under "refs/". when a new git user see >> the usage like, 'git push origin --all', we might feel like we're >> pushing _all_ the refs instead of just branches without looking at the >> documents until we found the related description of it or '--mirror'. > >Completely agree. > >This is something I spotted a long time ago. Although I would prefer >`--branches` over `--heads`. I will cook this recently, maybe we need some well prepared test cases at first. Thanks.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 5bb1d5aae25..a5d18fb90b6 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -147,6 +147,7 @@ already exists on the remote side. `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`. --all:: +--heads:: Push all branches (i.e. refs under `refs/heads/`); cannot be used with other <refspec>. diff --git a/builtin/push.c b/builtin/push.c index 60ac8017e52..970cabaa78b 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -588,11 +588,12 @@ int cmd_push(int argc, const char **argv, const char *prefix) struct option options[] = { OPT__VERBOSITY(&verbosity), OPT_STRING( 0 , "repo", &repo, N_("repository"), N_("repository")), - OPT_BIT( 0 , "all", &flags, N_("push all refs"), TRANSPORT_PUSH_ALL), + OPT_BIT( 0 , "all", &flags, N_("push all branches"), TRANSPORT_PUSH_ALL), + OPT_BIT( 0 , "heads", &flags, N_("push all branches"), TRANSPORT_PUSH_ALL), OPT_BIT( 0 , "mirror", &flags, N_("mirror all refs"), (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)), OPT_BOOL('d', "delete", &deleterefs, N_("delete refs")), - OPT_BOOL( 0 , "tags", &tags, N_("push tags (can't be used with --all or --mirror)")), + OPT_BOOL( 0 , "tags", &tags, N_("push tags (can't be used with --all or --heads or --mirror)")), OPT_BIT('n' , "dry-run", &flags, N_("dry run"), TRANSPORT_PUSH_DRY_RUN), OPT_BIT( 0, "porcelain", &flags, N_("machine-readable output"), TRANSPORT_PUSH_PORCELAIN), OPT_BIT('f', "force", &flags, N_("force updates"), TRANSPORT_PUSH_FORCE), @@ -635,7 +636,7 @@ int cmd_push(int argc, const char **argv, const char *prefix) set_push_cert_flags(&flags, push_cert); if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR)))) - die(_("options '%s' and '%s' cannot be used together"), "--delete", "--all/--mirror/--tags"); + die(_("options '%s' and '%s' cannot be used together"), "--delete", "--all/--heads/--mirror/--tags"); if (deleterefs && argc < 2) die(_("--delete doesn't make sense without any refs")); @@ -673,9 +674,9 @@ int cmd_push(int argc, const char **argv, const char *prefix) if (flags & TRANSPORT_PUSH_ALL) { if (tags) - die(_("options '%s' and '%s' cannot be used together"), "--all", "--tags"); + die(_("options '%s' and '%s' cannot be used together"), "--all/--heads", "--tags"); if (argc >= 2) - die(_("--all can't be combined with refspecs")); + die(_("--all/--heads can't be combined with refspecs")); } if (flags & TRANSPORT_PUSH_MIRROR) { if (tags) @@ -684,7 +685,7 @@ int cmd_push(int argc, const char **argv, const char *prefix) die(_("--mirror can't be combined with refspecs")); } if ((flags & TRANSPORT_PUSH_ALL) && (flags & TRANSPORT_PUSH_MIRROR)) - die(_("options '%s' and '%s' cannot be used together"), "--all", "--mirror"); + die(_("options '%s' and '%s' cannot be used together"), "--all/--heads", "--mirror"); if (!is_empty_cas(&cas) && (flags & TRANSPORT_PUSH_FORCE_IF_INCLUDES)) cas.use_force_if_includes = 1; diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh index fdb42920564..29d03b2f58b 100755 --- a/t/t5523-push-upstream.sh +++ b/t/t5523-push-upstream.sh @@ -60,13 +60,18 @@ test_expect_success 'push -u :topic_2' ' check_config topic_2 upstream refs/heads/other2 ' -test_expect_success 'push -u --all' ' - git branch all1 && - git branch all2 && - git push -u --all && - check_config all1 upstream refs/heads/all1 && - check_config all2 upstream refs/heads/all2 -' + +for option in 'all' 'heads' +do + + test_expect_success "push -u --$option" ' + git branch "$option"1 && + git branch "$option"2 && + git push -u --"$option" && + check_config "$option"1 upstream refs/heads/"$option"1 && + check_config "$option"2 upstream refs/heads/"$option"2 + ' +done test_expect_success 'push -u HEAD' ' git checkout -b headbranch &&