Re: [PATCH] submodule: add verbose mode for add/update
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:00:18
Orgad Shaneh [off-list ref] writes:
+--verbose:: + This option is valid for add and update commands. Display the progress + of the actual submodule checkout.
Hmm, is the "valid for add and update" part we want to keep? I do not think it is a crime if some other subcommand accepted --verbose option but its output under verbose mode and normal mode happened to be the same. I doubt it would take a lot of imagination to see that people would want to see "git submodule status --verbose" to get richer output, and at that point, "progress of checkout" as part of the description of the "--verbose" option does not make any sense. Perhaps the second part that is specific to "add" and "update" subcommands should move to the description of these two subcommands? I dunno.
quoted hunk
diff --git a/git-submodule.sh b/git-submodule.sh index a33f68d..e1df2c8 100755 --- a/git-submodule.sh +++ b/git-submodule.sh@@ -5,11 +5,11 @@ # Copyright (c) 2007 Lars Hjemli dashless=$(basename "$0" | sed -e 's/-/ /') -USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>] +USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [-v|--verbose] [--] <repository> [<path>] or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...] or: $dashless [--quiet] init [--] [<path>...] or: $dashless [--quiet] deinit [-f|--force] [--] <path>... - or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...] + or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [-v|--verbose] [--] [<path>...] or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...] or: $dashless [--quiet] foreach [--recursive] <command> or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"@@ -319,12 +319,16 @@ module_clone() rel=$(echo $a | sed -e 's|[^/][^/]*|..|g') ( clear_local_git_env + if test -z "$verbose" + then + subquiet=-q + fi cd "$sm_path" && GIT_WORK_TREE=. git config core.worktree "$rel/$b" && # ash fails to wordsplit ${local_branch:+-B "$local_branch"...} case "$local_branch" in - '') git checkout -f -q ${start_point:+"$start_point"} ;; - ?*) git checkout -f -q -B "$local_branch" ${start_point:+"$start_point"} ;; + '') git checkout -f ${subquiet:+"$subquiet"} ${start_point:+"$start_point"} ;; + ?*) git checkout -f ${subquiet:+"$subquiet"} -B "$local_branch" ${start_point:+"$start_point"} ;; esac ) || die "$(eval_gettext "Unable to setup cloned submodule '\$sm_path'")" }@@ -380,6 +384,9 @@ cmd_add() --depth=*) depth=$1 ;; + -v|--verbose) + verbose=1 + ;;
Compare $depth and $verbose, and think what would happen if the end user had an environment variable whose name happened to be $depth or $verbose. Does this script misbehave under such a stray $verbose? What does the existing script do to prevent it from misbehaving when a stray $depth exists in the environment?