Re: [PATCH] git-prompt.sh: add submodule indicator
I'm not well-versed in submodules, so I won't comment on whether this
is the right way to determine that a repository is a submodule, but I
was surprised to see how much you have to work to find this out.
My comments will mostly focus on how to eliminate or at least limit
the scope of subshells and command executions, because fork()s and
exec()s are rather expensive on Windows.
On Fri, Jan 20, 2017 at 1:07 AM, Benjamin Fuchs wrote:
> I expirienced that working with submodules can be confusing. This indicator
> will make you notice very easy when you switch into a submodule.
> The new prompt will look like this: (sub:master)
> Adding a new optional env variable for the new feature.
>
> Signed-off-by: Benjamin Fuchs
> ---
> contrib/completion/git-prompt.sh | 37 -
> 1 file changed, 36 insertions(+), 1 deletion(-)
Tests?
> diff --git a/contrib/completion/git-prompt.sh
> b/contrib/completion/git-prompt.sh
> index 97eacd7..4c82e7f 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -93,6 +93,10 @@
> # directory is set up to be ignored by git, then set
> # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
> # repository level by setting bash.hideIfPwdIgnored to "false".
> +#
> +# If you would like __git_ps1 to indicate that you are in a submodule,
> +# set GIT_PS1_SHOWSUBMODULE. In this case a "sub:" will be added before
> +# the branch name.
>
> # check whether printf supports -v
> __git_printf_supports_v=
> @@ -284,6 +288,32 @@ __git_eread ()
> test -r "$f" && read "$@" <"$f"
> }
>
> +# __git_is_submodule
> +# Based on:
> +#
> http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule
> +__git_is_submodule ()
Use the '__git_ps1' prefix in the function name, like the other
functions.
> +{
> + local git_dir parent_git module_name path
> + # Find the root of this git repo, then check if its parent dir is
> also a repo
> + git_dir="$(git rev-parse --show-toplevel)"
1. This is a somewhat misleading variable name, because git_dir (with
or without underscore or dash) refers to the path to the .git
directory of a repository through the codebase, documentation and
CLI options, not the top-level directory of the worktree.
2. In a bare repository or in the .git directory of a "regular"
repository '--show-toplevel' doesn't output anything, leading to
an empty $module_name below, which ultimately results in no
submodule indicator.
As fas as behavior is concerned, this is good in the bare
repository case, because as I understand it there is no such thing
as a bare submodule. I'm not sure whether the submodule indicator
should be displayed in the ".git dir of a submodule" case. I
leave it up to you and Stefan to discuss.
However, the info about whether we are in a bare repository or
.git dir is already availabe in certain variables, so we know
upfront when the current repository can't be a submodule. In
those cases this function should not be called only to spend
several subshells and command executions to figure out what we
already knew anyway.
3. The path to the .git directory of the current repository
is already available in the (not particularly descriptively named)
$g variable from __git_ps1. Perhaps you could use that variable
instead, thus avoiding a subshell and executing a git command
here.
> + module_name="$(basename "$git_dir")"
This is executed even when there is no repository in the parent
directories, but it's only needed when there _is_ a repo up there.
Please move it into the conditional below, to avoid a subshell and
command execution in the main code path.
Since this $git_dir comes directly from our own 'git rev-parse' do we
have to be prepared for a Windows-style path there? If it were always
a UNIX-style path, then we could strip all directories with shell
parameter expansion, eliminating both the subshell and the basename
execution.
> + parent_git="$(cd "$git_dir/.." && git rev-parse --show-toplevel 2>
> /dev/null)"
Style nit: no space after redirection, i.e. it should be '2>/dev/null'.
More importantly, I don't think you really need this variable:
1. The existence of a parent git repository can be determined from
'git rev-parse's exit code alone.
2. When you run 'git submodule' below, you don't have to cd to the
_top-level_ directory of the parent repository's worktree. You
just have to cd to _any_ directory in the parent, and you can do
that with 'git -C "$git_dir/.." submodule ...', without knowing
the parent's top-level directory.
This means that you don't need 'git rev-parse's output, thus there is
no need for the command substitution. Yet another subshell spared! :)
However, then you have to be careful with changing directories, and
should write it as 'git -C "$git_dir/.." rev-parse ...'.
> +
Re: [PATCH] git-prompt.sh: add submodule indicator
On Thu, Jan 19, 2017 at 4:07 PM, Benjamin Fuchs wrote:
> I expirienced that working with submodules can be confusing. This indicator
> will make you notice very easy when you switch into a submodule.
> The new prompt will look like this: (sub:master)
> Adding a new optional env variable for the new feature.
>
> Signed-off-by: Benjamin Fuchs
Thanks for making submodules better :)
Relevant tangent, just posted today:
https://public-inbox.org/git/20170119193023.26837-1-sbel...@google.com/T/#u
> ---
> contrib/completion/git-prompt.sh | 37 -
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-prompt.sh
> b/contrib/completion/git-prompt.sh
> index 97eacd7..4c82e7f 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -93,6 +93,10 @@
> # directory is set up to be ignored by git, then set
> # GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
> # repository level by setting bash.hideIfPwdIgnored to "false".
> +#
> +# If you would like __git_ps1 to indicate that you are in a submodule,
> +# set GIT_PS1_SHOWSUBMODULE. In this case a "sub:" will be added before
> +# the branch name.
>
> # check whether printf supports -v
> __git_printf_supports_v=
> @@ -284,6 +288,32 @@ __git_eread ()
> test -r "$f" && read "$@" <"$f"
> }
>
> +# __git_is_submodule
> +# Based on:
> +#
> http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule
> +__git_is_submodule ()
> +{
> + local git_dir parent_git module_name path
> + # Find the root of this git repo, then check if its parent dir is
> also a repo
> + git_dir="$(git rev-parse --show-toplevel)"
> + module_name="$(basename "$git_dir")"
> + parent_git="$(cd "$git_dir/.." && git rev-parse --show-toplevel 2>
> /dev/null)"
I wonder if we want to have better plumbing commands for submodules
here as well:
Here we only check if we have an embedded git repository, which may not be a
submodule. It could be a standalone repo that just happens to be inside another.
It could be a fake submodule [1], though I think the last time I
brought these up,
the upstream Git community considered these fake submodules are bug not worth
fixing.
And this doesn't cover the case that I addressed in the RFC-ish patch above:
$ git submodule deinit --all
$ cd && git status
# in an ideal world this tells you:
# "You are in an un-populated submodule. To change the submodule state..."
So I guess this is a good first approximation that actually gets most
of the cases right,
thereby helping a lot of people. But I wonder about these cornercases as well?
[1]
debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb
Thanks,
Stefan
[PATCH] git-prompt.sh: add submodule indicator
I expirienced that working with submodules can be confusing. This indicator
will make you notice very easy when you switch into a submodule.
The new prompt will look like this: (sub:master)
Adding a new optional env variable for the new feature.
Signed-off-by: Benjamin Fuchs
---
contrib/completion/git-prompt.sh | 37 -
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 97eacd7..4c82e7f 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -93,6 +93,10 @@
# directory is set up to be ignored by git, then set
# GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
# repository level by setting bash.hideIfPwdIgnored to "false".
+#
+# If you would like __git_ps1 to indicate that you are in a submodule,
+# set GIT_PS1_SHOWSUBMODULE. In this case a "sub:" will be added before
+# the branch name.
# check whether printf supports -v
__git_printf_supports_v=
@@ -284,6 +288,32 @@ __git_eread ()
test -r "$f" && read "$@" <"$f"
}
+# __git_is_submodule
+# Based on:
+#
http://stackoverflow.com/questions/7359204/git-command-line-know-if-in-submodule
+__git_is_submodule ()
+{
+ local git_dir parent_git module_name path
+ # Find the root of this git repo, then check if its parent dir is also
a repo
+ git_dir="$(git rev-parse --show-toplevel)"
+ module_name="$(basename "$git_dir")"
+ parent_git="$(cd "$git_dir/.." && git rev-parse --show-toplevel 2>
/dev/null)"
+ if [[ -n $parent_git ]]; then
+ # List all the submodule paths for the parent repo
+ while read path
+ do
+ if [[ "$path" != "$module_name" ]]; then continue; fi
+ if [[ -d "$git_dir/../$path" ]];then return 0; fi
+ done < <(cd $parent_git && git submodule --quiet foreach 'echo
$path' 2> /dev/null)
+fi
+return 1
+}
+
+__git_ps1_submodule ()
+{
+ __git_is_submodule && printf "sub:"
+}
+
# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
# when called from PS1 using command substitution
# in this mode it prints text to add to bash PS1 prompt (includes branch name)
@@ -513,8 +543,13 @@ __git_ps1 ()
b="\${__git_ps1_branch_name}"
fi
+ local sub=""
+ if [ -n "${GIT_PS1_SHOWSUBMODULE}" ]; then
+ sub="$(__git_ps1_submodule)"
+ fi
+
local f="$w$i$s$u"
- local gitstring="$c$b${f:+$z$f}$r$p"
+ local gitstring="$c$sub$b${f:+$z$f}$r$p"
if [ $pcmode = yes ]; then
if [ "${__git_printf_supports_v-}" != yes ]; then
--
2.7.4
