{"thread":{"id":"44920","subject":"[PATCH] git-prompt.sh: add submodule indicator","startedAt":"2017-01-20T00:14:43Z","lastAt":"2017-01-20T13:36:39Z","messageCount":3,"participants":["Benjamin Fuchs","Stefan Beller","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"309810","messageId":"1484870858-6336-1-git-send-email-email@benjaminfuchs.de","threadId":"44920","inReplyTo":null,"subject":"[PATCH] git-prompt.sh: add submodule indicator","fromName":"Benjamin Fuchs","fromEmail":"email@benjaminfuchs.de","sentAt":"2017-01-20T00:07:38Z","receivedAt":"2017-01-20T00:14:43Z","isPatch":true,"sender":{"key":"email@benjaminfuchs.de","avatar":null},"body":"I expirienced that working with submodules can be confusing. This indicator\nwill make you notice very easy when you switch into a submodule.\nThe new prompt will look like this: (sub:master)\nAdding a new optional env variable for the new feature.\n\nSigned-off-by: Benjamin Fuchs <email@benjaminfuchs.de>\n---\n contrib/completion/git-prompt.sh | 37 ++++++++++++++++++++++++++++++++++++-\n 1 file changed, 36 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\nindex 97eacd7..4c82e7f 100644\n--- a/contrib/completion/git-prompt.sh\n+++ b/contrib/completion/git-prompt.sh\n@@ -93,6 +93,10 @@\n # directory is set up to be ignored by git, then set\n # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the\n # repository level by setting bash.hideIfPwdIgnored to \"false\".\n+#\n+# If you would like __git_ps1 to indicate that you are in a submodule,\n+# set GIT_PS1_SHOWSUBMODULE. In this case a \"sub:\" will be added before\n+# the branch name.\n \n # check whether printf supports -v\n __git_printf_supports_v=\n@@ -284,6 +288,32 @@ __git_eread ()\n \ttest -r \"$f\" && read \"$@\" <\"$f\"\n }\n \n+# __git_is_submodule\n+# Based on:\n+# http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule\n+__git_is_submodule ()\n+{\n+\tlocal git_dir parent_git module_name path\n+\t# Find the root of this git repo, then check if its parent dir is also a repo\n+\tgit_dir=\"$(git rev-parse --show-toplevel)\"\n+\tmodule_name=\"$(basename \"$git_dir\")\"\n+\tparent_git=\"$(cd \"$git_dir/..\" && git rev-parse --show-toplevel 2> /dev/null)\"\n+\tif [[ -n $parent_git ]]; then\n+\t\t# List all the submodule paths for the parent repo\n+\t\twhile read path\n+\t\tdo\n+\t\t\tif [[ \"$path\" != \"$module_name\" ]]; then continue; fi\n+\t\t\tif [[ -d \"$git_dir/../$path\" ]];    then return 0; fi\n+\t\tdone < <(cd $parent_git && git submodule --quiet foreach 'echo $path' 2> /dev/null)\n+    fi\n+    return 1\n+}\n+\n+__git_ps1_submodule ()\n+{\n+\t__git_is_submodule && printf \"sub:\"\n+}\n+\n # __git_ps1 accepts 0 or 1 arguments (i.e., format string)\n # when called from PS1 using command substitution\n # in this mode it prints text to add to bash PS1 prompt (includes branch name)\n@@ -513,8 +543,13 @@ __git_ps1 ()\n \t\tb=\"\\${__git_ps1_branch_name}\"\n \tfi\n \n+\tlocal sub=\"\"\n+\tif [ -n \"${GIT_PS1_SHOWSUBMODULE}\" ]; then\n+\t\tsub=\"$(__git_ps1_submodule)\"\n+\tfi\n+\n \tlocal f=\"$w$i$s$u\"\n-\tlocal gitstring=\"$c$b${f:+$z$f}$r$p\"\n+\tlocal gitstring=\"$c$sub$b${f:+$z$f}$r$p\"\n \n \tif [ $pcmode = yes ]; then\n \t\tif [ \"${__git_printf_supports_v-}\" != yes ]; then\n-- \n2.7.4\n\n"},{"id":"309812","messageId":"CAGZ79kZNq=O=z-OSr-vDL4SUNViAUZ7AWz01TkGrmG88ZNqxgg@mail.gmail.com","threadId":"44920","inReplyTo":"1484870858-6336-1-git-send-email-email@benjaminfuchs.de","subject":"Re: [PATCH] git-prompt.sh: add submodule indicator","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-20T00:28:07Z","receivedAt":"2017-01-20T00:38:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jan 19, 2017 at 4:07 PM, Benjamin Fuchs <email@benjaminfuchs.de> wrote:\n> I expirienced that working with submodules can be confusing. This indicator\n> will make you notice very easy when you switch into a submodule.\n> The new prompt will look like this: (sub:master)\n> Adding a new optional env variable for the new feature.\n>\n> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>\n\nThanks for making submodules better :)\nRelevant tangent, just posted today:\nhttps://public-inbox.org/git/20170119193023.26837-1-sbeller@google.com/T/#u\n\n> ---\n>  contrib/completion/git-prompt.sh | 37 ++++++++++++++++++++++++++++++++++++-\n>  1 file changed, 36 insertions(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\n> index 97eacd7..4c82e7f 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -93,6 +93,10 @@\n>  # directory is set up to be ignored by git, then set\n>  # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the\n>  # repository level by setting bash.hideIfPwdIgnored to \"false\".\n> +#\n> +# If you would like __git_ps1 to indicate that you are in a submodule,\n> +# set GIT_PS1_SHOWSUBMODULE. In this case a \"sub:\" will be added before\n> +# the branch name.\n>\n>  # check whether printf supports -v\n>  __git_printf_supports_v=\n> @@ -284,6 +288,32 @@ __git_eread ()\n>         test -r \"$f\" && read \"$@\" <\"$f\"\n>  }\n>\n> +# __git_is_submodule\n> +# Based on:\n> +# http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule\n> +__git_is_submodule ()\n> +{\n> +       local git_dir parent_git module_name path\n> +       # Find the root of this git repo, then check if its parent dir is also a repo\n> +       git_dir=\"$(git rev-parse --show-toplevel)\"\n> +       module_name=\"$(basename \"$git_dir\")\"\n> +       parent_git=\"$(cd \"$git_dir/..\" && git rev-parse --show-toplevel 2> /dev/null)\"\n\nI wonder if we want to have better plumbing commands for submodules\nhere as well:\nHere we only check if we have an embedded git repository, which may not be a\nsubmodule. It could be a standalone repo that just happens to be inside another.\nIt could be a fake submodule [1], though I think the last time I\nbrought these up,\nthe upstream Git community considered these fake submodules are bug not worth\nfixing.\n\nAnd this doesn't cover the case that I addressed in the RFC-ish patch above:\n  $ git submodule deinit --all\n  $ cd <submodule> && git status\n  # in an ideal world this tells you:\n  #  \"You are in an un-populated submodule. To change the submodule state...\"\n\nSo I guess this is a good first approximation that actually gets most\nof the cases right,\nthereby helping a lot of people. But I wonder about these cornercases as well?\n\n[1] debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb\n\nThanks,\nStefan\n"},{"id":"309826","messageId":"CAM0VKjmkOL6Ty=KLUbuV2CZao7x1YF_+X82JxyX-8gchi6Dzhg@mail.gmail.com","threadId":"44920","inReplyTo":"1484870858-6336-1-git-send-email-email@benjaminfuchs.de","subject":"Re: [PATCH] git-prompt.sh: add submodule indicator","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2017-01-20T13:35:27Z","receivedAt":"2017-01-20T13:36:39Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"I'm not well-versed in submodules, so I won't comment on whether this\nis the right way to determine that a repository is a submodule, but I\nwas surprised to see how much you have to work to find this out.\n\nMy comments will mostly focus on how to eliminate or at least limit\nthe scope of subshells and command executions, because fork()s and\nexec()s are rather expensive on Windows.\n\nOn Fri, Jan 20, 2017 at 1:07 AM, Benjamin Fuchs <email@benjaminfuchs.de> wrote:\n> I expirienced that working with submodules can be confusing. This indicator\n> will make you notice very easy when you switch into a submodule.\n> The new prompt will look like this: (sub:master)\n> Adding a new optional env variable for the new feature.\n>\n> Signed-off-by: Benjamin Fuchs <email@benjaminfuchs.de>\n> ---\n>  contrib/completion/git-prompt.sh | 37 ++++++++++++++++++++++++++++++++++++-\n>  1 file changed, 36 insertions(+), 1 deletion(-)\n\nTests?\n\n> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh\n> index 97eacd7..4c82e7f 100644\n> --- a/contrib/completion/git-prompt.sh\n> +++ b/contrib/completion/git-prompt.sh\n> @@ -93,6 +93,10 @@\n>  # directory is set up to be ignored by git, then set\n>  # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the\n>  # repository level by setting bash.hideIfPwdIgnored to \"false\".\n> +#\n> +# If you would like __git_ps1 to indicate that you are in a submodule,\n> +# set GIT_PS1_SHOWSUBMODULE. In this case a \"sub:\" will be added before\n> +# the branch name.\n>\n>  # check whether printf supports -v\n>  __git_printf_supports_v=\n> @@ -284,6 +288,32 @@ __git_eread ()\n>         test -r \"$f\" && read \"$@\" <\"$f\"\n>  }\n>\n> +# __git_is_submodule\n> +# Based on:\n> +# http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule\n> +__git_is_submodule ()\n\nUse the '__git_ps1' prefix in the function name, like the other\nfunctions.\n\n> +{\n> +       local git_dir parent_git module_name path\n> +       # Find the root of this git repo, then check if its parent dir is also a repo\n> +       git_dir=\"$(git rev-parse --show-toplevel)\"\n\n 1. This is a somewhat misleading variable name, because git_dir (with\n    or without underscore or dash) refers to the path to the .git\n    directory of a repository through the codebase, documentation and\n    CLI options, not the top-level directory of the worktree.\n\n 2. In a bare repository or in the .git directory of a \"regular\"\n    repository '--show-toplevel' doesn't output anything, leading to\n    an empty $module_name below, which ultimately results in no\n    submodule indicator.\n\n    As fas as behavior is concerned, this is good in the bare\n    repository case, because as I understand it there is no such thing\n    as a bare submodule.  I'm not sure whether the submodule indicator\n    should be displayed in the \".git dir of a submodule\" case.  I\n    leave it up to you and Stefan to discuss.\n\n    However, the info about whether we are in a bare repository or\n    .git dir is already availabe in certain variables, so we know\n    upfront when the current repository can't be a submodule.  In\n    those cases this function should not be called only to spend\n    several subshells and command executions to figure out what we\n    already knew anyway.\n\n 3. The path to the .git directory of the current repository\n    is already available in the (not particularly descriptively named)\n    $g variable from __git_ps1.  Perhaps you could use that variable\n    instead, thus avoiding a subshell and executing a git command\n    here.\n\n> +       module_name=\"$(basename \"$git_dir\")\"\n\nThis is executed even when there is no repository in the parent\ndirectories, but it's only needed when there _is_ a repo up there.\nPlease move it into the conditional below, to avoid a subshell and\ncommand execution in the main code path.\n\nSince this $git_dir comes directly from our own 'git rev-parse' do we\nhave to be prepared for a Windows-style path there?  If it were always\na UNIX-style path, then we could strip all directories with shell\nparameter expansion, eliminating both the subshell and the basename\nexecution.\n\n> +       parent_git=\"$(cd \"$git_dir/..\" && git rev-parse --show-toplevel 2> /dev/null)\"\n\nStyle nit: no space after redirection, i.e. it should be '2>/dev/null'.\n\nMore importantly, I don't think you really need this variable:\n\n 1. The existence of a parent git repository can be determined from\n    'git rev-parse's exit code alone.\n\n 2. When you run 'git submodule' below, you don't have to cd to the\n    _top-level_ directory of the parent repository's worktree.  You\n    just have to cd to _any_ directory in the parent, and you can do\n    that with 'git -C \"$git_dir/..\" submodule ...', without knowing\n    the parent's top-level directory.\n\nThis means that you don't need 'git rev-parse's output, thus there is\nno need for the command substitution.  Yet another subshell spared! :)\nHowever, then you have to be careful with changing directories, and\nshould write it as 'git -C \"$git_dir/..\" rev-parse ...'.\n\n> +       if [[ -n $parent_git ]]; then\n\nUnquoted path.\n\n> +               # List all the submodule paths for the parent repo\n> +               while read path\n> +               do\n> +                       if [[ \"$path\" != \"$module_name\" ]]; then continue; fi\n> +                       if [[ -d \"$git_dir/../$path\" ]];    then return 0; fi\n> +               done < <(cd $parent_git && git submodule --quiet foreach 'echo $path' 2> /dev/null)\n\nUnquoted path and space after redirection again.\n\nThis loop will get confused if one of the submodule paths contains a\nnewline, though probably no one in their right mind would do something\nlike that.  Is it even possible to have a submodule name with newline\nin it?\nI wonder whether it would be possible to move the conditions from the\nloop body to the shell command evaluated by 'submodule foreach', thus\neliminating the loop entirely and making submodule detection\nnewline-safe.  Or whether it's worth the trouble...\n\n> +    fi\n> +    return 1\n\nIndent with spaces, please use tabs instead.\n\n> +}\n> +\n> +__git_ps1_submodule ()\n> +{\n> +       __git_is_submodule && printf \"sub:\"\n> +}\n\nThis function is unnecessary.\nFirst, it's so short that it's body can be safely inlined in its\ncaller.  Second, it _prints_ its output to stdout, forcing the\ncaller to use a command substitution.  You can simply assign the value\nto $sub based on __git_ps1_is_submodule()'s return value.\n\n> +\n>  # __git_ps1 accepts 0 or 1 arguments (i.e., format string)\n>  # when called from PS1 using command substitution\n>  # in this mode it prints text to add to bash PS1 prompt (includes branch name)\n> @@ -513,8 +543,13 @@ __git_ps1 ()\n>                 b=\"\\${__git_ps1_branch_name}\"\n>         fi\n>\n> +       local sub=\"\"\n\nThank you for _not_ introducing yet another one letter variable name\n:)\n\n> +       if [ -n \"${GIT_PS1_SHOWSUBMODULE}\" ]; then\n> +               sub=\"$(__git_ps1_submodule)\"\n> +       fi\n> +\n>         local f=\"$w$i$s$u\"\n> -       local gitstring=\"$c$b${f:+$z$f}$r$p\"\n> +       local gitstring=\"$c$sub$b${f:+$z$f}$r$p\"\n>\n>         if [ $pcmode = yes ]; then\n>                 if [ \"${__git_printf_supports_v-}\" != yes ]; then\n> --\n> 2.7.4\n>\n"}]}