Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: [PATCH] vcs_info git: fix applied patch detection on git am



On 9 October 2014 00:24, Marc Finet <m.dreadlock@xxxxxxxxx> wrote:
> git-am also uses .git/rebase-apply for patch list but
> the file original-commit does not exist (as no commit exist).
> This patch handles both git rebase and git am. Also:
>  - get the first line (rather than the first char) when the message
>    contains only one line;
>  - remove unused function (ironically that should have been used here).
> ---
>  Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
> index ee50be6..48d552f 100644
> --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
> +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
> @@ -195,14 +195,6 @@ fi
>  VCS_INFO_adjust
>  VCS_INFO_git_getaction ${gitdir}
>
> -
> -VCS_INFO_get_get_rebase()
> -{
> -    if [[ -f "$1" ]]; then
> -       echo "$(< "$1")"
> -    fi
> -}
> -
>  local patchdir=${gitdir}/patches/${gitbranch}
>  if [[ -d $patchdir ]] && [[ -f $patchdir/applied ]] \
>     && [[ -f $patchdir/unapplied ]]
> @@ -223,11 +215,16 @@ elif [[ -d "${gitdir}/rebase-apply" ]]; then
>      # Fake patch names for all but current patch
>      patchdir="${gitdir}/rebase-apply"
>      local cur=$(< "${patchdir}/next")
> -    local p
> +    local p subject
>      for p in $(seq $(($cur - 1))); do
>          git_patches_applied+=("$(printf "%04d" $p) ?")
>      done
> -    git_patches_applied+=("$(< "${patchdir}/original-commit") ${${(f)$(< "${patchdir}/msg-clean")}[1]}")
> +    subject="${$(< "${patchdir}/msg-clean")[(f)1]}"
> +    if [[ -f "${patchdir}/original-commit" ]]; then
> +        git_patches_applied+=("$(< ${patchdir}/original-commit) $subject")
> +    else
> +        git_patches_applied+=("? $subject")
> +    fi
>      git_patches_unapplied=($(seq $cur $(< "${patchdir}/last")))
>
>      VCS_INFO_git_handle_patches
> --
> 2.1.1
>

We should possibly have a -f check for every instance of $(< since
this construct will abort script execution if the file does not exist,
eg
echo $(<nonexist); echo hello
will never print hello, whereas
echo $(cat nonexist); echo hello
would.

I encountered the particular problem you're fixing, and when it
happens it results in not only an error message, but no git info being
displayed at all, and it would be nice to be more robust against
unexpected changes in how git works.

(This is more of a general note, I think your patch is fine as it is).

-- 
Mikael Magnusson



Messages sorted by: Reverse Date, Date, Thread, Author