Download raw body.
got branch: support multiple -d options
resuming the thread. Sorry Stefan for posting almost the same diff you sent back then, it wasn't on purpose, i completely forgot about this thread and ended up reinventing it to scratch an itch. Omar Polo <op@omarpolo.com> wrote: > Stefan Sperling <stsp@stsp.name> wrote: > > On Sun, May 15, 2022 at 11:22:28PM +0200, Christian Weisgerber wrote: > > > Stefan Sperling: > > > > > > > If multiple -d options are passed to 'got branch' all arguments > > > > except the last one end up being ignored. > > > > > > > > It seems more useful to delete all specified branches instead. > > > > > > Hmmm. Should this behave like "ref -d", where the -d flag doesn't take > > > an argument but switches to delete mode? > > > > Or would it make sense to adjust 'got ref -d' to behave like > > 'got send -d' and 'got branch -d' (after this patch)? > > FWIW I'm fine either the way but after hearing naddy I think I agree > with him. Having -d as a "mode switch" allows to type something like > > $ got branch -d foo bar baz ... > > which is handier than spamming -d ... -d ... -d ... but admittedly i > don't remember how many times I wanted to delete more than one branch. > > `got send -d' can't be tweaked to have -d as a "mode switch" because it > needs already a positional argument for the remote to use, but that's > probably fine as is. even tho i just sent a diff that's almost identical to the one Stefan sent, re-reading the thread convinced me that i agree with naddy and the past myself. There isn't much point in using -d in combo with other flags, and even if it were it's currently disallowed. on the other hand, a "mode switch" -d is handy because one don't have to spam multiple flags when cleaning up some temporary branches. i'm attaching (yet another) diff that makes -d a switch. the only downside i see is that the description becomes a bit more weird since branch allows only one positional argument unless -d is given. I don't see a way to make it cleaner in the usage string (i.e. `got branch [-d] [name | branches...]' doesn't read well imho) so i just opted to add a note that in case -d is provided, more than one branch name can be given. i'm still expecting that 90% of the usage will just be to delete a single branch, no need to make the usage string overly complex. note also that usage_branch already show [-d] and not [-d branch] as the manpage did :) > > Perhaps 'got ref' could remain as it is. Since it is not intended to > > be used on a daily basis it doesn't need to provide much comfort. > > Unlike the other tools it requires absolute refnames as input (which > > start with "refs/"), and it does not have any safety checks. For instance, > > 'got branch -d' will refuse to delete a branch that is used by the current > > work tree. 'got ref -d' will delete the branch no matter what. no opinions about got ref -d, but i agree that it doesn't need to provide much comfort. the churn in branch.sh is mostly because now that -d is a mode switch (is it a real term or did i made that up?) the arguments needed to be sorted. diff /home/op/w/got commit - c5b5bb9d8b7eb7e70c0ab60736b349128a267edc path + /home/op/w/got blob - 918d723a3556ec33343925c453566d54da948b5f file + got/got.1 --- got/got.1 +++ got/got.1 @@ -1068,7 +1068,7 @@ Cannot be used together with any other options except .Fl r . .El .Tg br -.It Cm branch Oo Fl c Ar commit Oc Oo Fl r Ar repository-path Oc Oo Fl l Oc Oo Fl t Oc Oo Fl d Ar name Oc Oo Fl n Oc Op Ar name +.It Cm branch Oo Fl c Ar commit Oc Oo Fl r Ar repository-path Oc Oo Fl l Oc Oo Fl t Oc Oo Fl d Oc Oo Fl n Oc Op Ar name .Dl Pq alias: Cm br Create, list, or delete branches. .Pp @@ -1155,7 +1155,7 @@ regardless. Use of this option requires the .Fl l option to be used as well. -.It Fl d Ar name +.It Fl d Delete the branch with the specified .Ar name from the @@ -1163,6 +1163,9 @@ from the or .Dq refs/remotes reference namespace. +When using this option, more than one branch +.Ar name +to delete can be provided. .Pp Only the branch reference is deleted. Any commit, tree, and blob objects belonging to the branch blob - 009c0d06e2e5d8714a10c5c8a008771349f971b0 file + got/got.c --- got/got.c +++ got/got.c @@ -6675,7 +6675,8 @@ cmd_branch(int argc, char *argv[]) struct got_worktree *worktree = NULL; char *cwd = NULL, *repo_path = NULL; int ch, do_list = 0, do_show = 0, do_update = 1, sort_by_time = 0; - const char *delref = NULL, *commit_id_arg = NULL; + int do_delete = 0; + const char *commit_id_arg = NULL; struct got_reference *ref = NULL; struct got_pathlist_head paths; struct got_pathlist_entry *pe; @@ -6685,13 +6686,13 @@ cmd_branch(int argc, char *argv[]) TAILQ_INIT(&paths); - while ((ch = getopt(argc, argv, "c:d:r:lnt")) != -1) { + while ((ch = getopt(argc, argv, "c:dr:lnt")) != -1) { switch (ch) { case 'c': commit_id_arg = optarg; break; case 'd': - delref = optarg; + do_delete = 1; break; case 'r': repo_path = realpath(optarg, NULL); @@ -6715,7 +6716,7 @@ cmd_branch(int argc, char *argv[]) } } - if (do_list && delref) + if (do_list && do_delete) option_conflict('l', 'd'); if (sort_by_time && !do_list) errx(1, "-t option requires -l option"); @@ -6723,17 +6724,16 @@ cmd_branch(int argc, char *argv[]) argc -= optind; argv += optind; - if (!do_list && !delref && argc == 0) + if (!do_list && !do_delete && argc == 0) do_show = 1; - if ((do_list || delref || do_show) && commit_id_arg != NULL) + if ((do_list || do_delete || do_show) && commit_id_arg != NULL) errx(1, "-c option can only be used when creating a branch"); - if (do_list || delref) { - if (argc > 0) - usage_branch(); - } else if (!do_show && argc != 1) + if ((do_list || do_show) && argc > 0) usage_branch(); + else if (do_delete && argc < 1) + usage_branch(); #ifndef PROFILE if (pledge("stdio rpath wpath cpath fattr flock proc exec " @@ -6794,9 +6794,13 @@ cmd_branch(int argc, char *argv[]) error = show_current_branch(repo, worktree); else if (do_list) error = list_branches(repo, worktree, sort_by_time); - else if (delref) - error = delete_branch(repo, worktree, delref); - else { + else if (do_delete) { + for (; *argv; ++argv) { + error = delete_branch(repo, worktree, *argv); + if (error) + break; + } + } else { struct got_reflist_head refs; TAILQ_INIT(&refs); error = got_ref_list(&refs, repo, NULL, got_ref_cmp_by_name, blob - a025c5170abf06b7f707406ee2d9193c3fa6ff43 file + regress/cmdline/branch.sh --- regress/cmdline/branch.sh +++ regress/cmdline/branch.sh @@ -248,7 +248,7 @@ test_branch_delete() { fi done - got branch -d branch2 -r $testroot/repo > $testroot/stdout + got branch -r $testroot/repo -d branch2 > $testroot/stdout ret=$? if [ $ret -ne 0 ]; then echo "got branch command failed unexpectedly" @@ -281,7 +281,7 @@ test_branch_delete() { return 1 fi - got branch -d bogus_branch_name -r $testroot/repo \ + got branch -r $testroot/repo -d bogus_branch_name \ > $testroot/stdout 2> $testroot/stderr ret=$? if [ $ret -eq 0 ]; then @@ -323,7 +323,8 @@ test_branch_delete() { return 1 fi - got branch -d origin/branch1 -r $testroot/repo > $testroot/stdout + got branch -r $testroot/repo -d origin/branch1 \ + refs/remotes/origin/branch3 > $testroot/stdout ret=$? if [ $ret -ne 0 ]; then echo "got branch command failed unexpectedly" @@ -331,15 +332,6 @@ test_branch_delete() { return 1 fi - got branch -d refs/remotes/origin/branch3 -r $testroot/repo \ - > $testroot/stdout - ret=$? - if [ $ret -ne 0 ]; then - echo "got branch command failed unexpectedly" - test_done "$testroot" "$ret" - return 1 - fi - got ref -l -r $testroot/repo > $testroot/stdout echo "HEAD: refs/heads/master" > $testroot/stdout.expected echo "refs/heads/branch1: $commit_id" >> $testroot/stdout.expected @@ -396,7 +388,7 @@ test_branch_delete_packed() { (cd $testroot/repo && git pack-refs --all) - got branch -d refs/heads/branch2 -r $testroot/repo > $testroot/stdout + got branch -r $testroot/repo -d refs/heads/branch2 > $testroot/stdout ret=$? if [ $ret -ne 0 ]; then echo "got update command failed unexpectedly" @@ -429,7 +421,7 @@ test_branch_delete_packed() { return 1 fi - got branch -d bogus_branch_name -r $testroot/repo \ + got branch -r $testroot/repo -d bogus_branch_name \ > $testroot/stdout 2> $testroot/stderr ret=$? if [ $ret -eq 0 ]; then
got branch: support multiple -d options