Administrivia:
- "Content-Type: text/plain; charset=ISO-8859-1; format=flowed" in
the header tells us that the patch is corrupted and will not
apply cleanly, but we still can discuss the contents of the
patch.
- "git shortlog --since=6.months -n --no-merges contrib/completion"
tells us who the area experts who may be able to help you move
this patch forward. Also people who are involved in git-svn may
be able to help with the configurations you are reading from. By
Cc'ing them, your patch may have a better chance of getting
polished more quickly (I added Cc to relevant people).
Yves Blusseau <blusseau@xxxxxxxxx> writes:
> Updated version of my previous patch
Everything before the "---" line we see below will be for people who
read "git log" history 6 months down the road, when they have to
understand what you did and why (perhaps because they found bugs or
they want to further enhance what your patch did).
Because your previous patch will not appear in that "git log"
output, the above is not an appropriate thing to say here.
> This fix is used to return the svn reference of the remote svn upstream
> branch when the git repository is a clone of a svn repository that was
> created with the --stdlayout and --prefix options of git svn command.
Before using the word "fix", please describe what the current
(i.e. without the "fix") behaviour visible to the users, why it is
bad, and what behaviour you want to give your users instead. And
then describe what change you made to give that desired behaviour.
> * completion/git-prompt.sh: add function to resolve svn branches into
> git remote refs using paths declared in svn-remote.*.fetch and
> svn-remote.*.branches configurations
>
> Signed-off-by: Yves Blusseau <blusseau@xxxxxxxxx>
> ---
Here, immediately after "---", is where you would say "Updated
version of my patch. The difference from the previous round are
1. fixed foo, 2. fixed bar, 3. reworded baz...." if you want to.
> contrib/completion/git-prompt.sh | 56
> +++++++++++++++++++++++++++++++++++++++-
Line-wrapped (comes from "format=flawed" we saw in the Content-Type
header).
> 1 file changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-prompt.sh
> b/contrib/completion/git-prompt.sh
> index 29b1ec9..f9a3421 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -73,17 +73,65 @@ __gitdir ()
> fi
> }
> +# resolve svn refs
> +# The function accepts 2 arguments:
> +# 1: An array containing the translation ref(s) (ie
> branches/*:refs/remotes/svn/*)
> +# 2: the ref to be translated (ie. branches/prod)
> +# returns the remote refs or original ref if it could not be translated
We see "translation ref(s)", which is an unheard-of term, and it is
unclear what is translated and into what other thing. Is this the
moral equivalent of the ref mapping that maps refs at the remote
repository to remote tracking refs we have at our local repository?
If the first item of above were written like this:
# 1: An array containing the mapping from branches in the subversion
# repository to remote tracking branches in this repository (e.g
# "branches/*:refs/remotes/svn/*").
I wouldn't have had to ask the above question, but it is unclear if
that is what you meant, hence this question.
> +__git_resolve_svn_refs ()
> +{
> + local idx ref_globs left_part_ref_glob right_part_ref_glob
> left right value
> +
> + local all_ref_globs=("${!1}")
> + local ref="$2"
Bash/Zsh reusability czars might want to comment on the way
all_ref_globs is declared and initialized.
> + for (( idx=0 ; idx < ${#all_ref_globs[@]}; idx++ ));do
> + ref_globs="${all_ref_globs[$idx]}"
> + # get the left part ref glob (before the :refs/)
> + left_part_ref_glob="${ref_globs%%:refs/*}"
Is doubling of %% significant here? I am assuming there isn't, as
there would be no colon in $ref_globs other than the boundary
between LHS and RHS.
> + # If the ref match the left part we can resolve the ref directly
> + if [[ "$ref" == "$left_part_ref_glob" ]];then
The coding standard used in this file is kept deliberately lax
compared to the remainder of the codebase, but I think the lack of
SP between ";" and "then" goes a bit too far.
> + # resolve the ref into the right part (after the :)
> + ref="${ref_globs/${left_part_ref_glob}:/}"
> + break
> + else
> + case $ref in
> + # check if the ref match the glob
> + $left_part_ref_glob)
case/esac and case arms should align at the same column (otherwise
the body will be indented unreadably too deep to the right, like
this code).
> # stores the divergence from upstream in $p
> # used by GIT_PS1_SHOWUPSTREAM
> __git_ps1_show_upstream ()
> {
> local key value
> local svn_remote svn_url_pattern count n
> + local svn_refs_globs=()
> local upstream=git legacy="" verbose=""
> svn_remote=()
Doesn't this have the same problem as the one fixed by 471dcfd
(contrib/completion: "local var=()" is misinterpreted as func-decl
by zsh, 2011-09-01)?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html