Re: [PATCH] completion: clear cached --options when sourcing the completion script

[Duy Nguyen <pclouds@gmail.com>]

On Thu, Mar 22, 2018 at 3:16 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> The established way to update the completion script in an already
> running shell is to simply source it again: this brings in any new
> --options and features, and clears caching variables.  E.g. it clears
> the variables caching the list of (all|porcelain) git commands, so
> when they are later lazy-initialized again, then they will list and
> cache any newly installed commmands as well.
>
> Unfortunately, since d401f3debc (git-completion.bash: introduce
> __gitcomp_builtin, 2018-02-09) and subsequent patches this doesn't
> work for a lot of git commands' options.  To eliminate a lot of
> hard-to-maintain hard-coded lists of options, those commits changed
> the completion script to use a bunch of programmatically created and
> lazy-initialized variables to cache the options of those builtin
> porcelain commands that use parse-options.  These variables are not
> cleared upon sourcing the completion script, therefore they continue
> caching the old lists of options, even when some commands recently
> learned new options or when deprecated options were removed.
>
> Always 'unset' these variables caching the options of builtin commands
> when sourcing the completion script.

And here I have been happily unsetting these manually when I re-source
to test stuff, not thinking it as a bug. Thanks!

> Redirect 'unset's stderr to /dev/null, because ZSH's 'unset' complains
> if it's invoked without any arguments, i.e. no variables caching
> builtin's options are set.  This can happen, if someone were to source
> the completion script twice without completing any --options in
> between.  Bash stays silent in this case.
>
> Add tests to ensure that these variables are indeed cleared when the
> completion script is sourced; not just the variables caching options,
> but all other caching variables, i.e. the variables caching commands,
> porcelain commands and merge strategies as well.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>
> Fixes a recent regression introduced in 'nd/parseopt-completion'.
>
>  contrib/completion/git-completion.bash |  4 ++++
>  t/t9902-completion.sh                  | 31 ++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 7c84eb1912..602352f952 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -280,6 +280,10 @@ __gitcomp ()
>         esac
>  }
>
> +# Clear the variables caching builtins' options when (re-)sourcing
> +# the completion script.
> +unset $(set |sed -ne 's/^\(__gitcomp_builtin_[a-zA-Z0-9_][a-zA-Z0-9_]*\)=.*/\1/p') 2>/dev/null
> +
>  # This function is equivalent to
>  #
>  #    __gitcomp "$(git xxx --git-completion-helper) ..."
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index e6485feb0a..4c86adadf2 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1497,4 +1497,35 @@ do
>         '
>  done
>
> +test_expect_success 'sourcing the completion script clears cached commands' '
> +       __git_compute_all_commands &&
> +       verbose test -n "$__git_all_commands" &&
> +       . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> +       verbose test -z "$__git_all_commands"
> +'
> +
> +test_expect_success 'sourcing the completion script clears cached porcelain commands' '
> +       __git_compute_porcelain_commands &&
> +       verbose test -n "$__git_porcelain_commands" &&
> +       . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> +       verbose test -z "$__git_porcelain_commands"
> +'
> +
> +test_expect_success 'sourcing the completion script clears cached merge strategies' '
> +       __git_compute_merge_strategies &&
> +       verbose test -n "$__git_merge_strategies" &&
> +       . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> +       verbose test -z "$__git_merge_strategies"
> +'
> +
> +test_expect_success 'sourcing the completion script clears cached --options' '
> +       __gitcomp_builtin checkout &&
> +       verbose test -n "$__gitcomp_builtin_checkout" &&
> +       __gitcomp_builtin notes_edit &&
> +       verbose test -n "$__gitcomp_builtin_notes_edit" &&
> +       . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> +       verbose test -z "$__gitcomp_builtin_checkout" &&
> +       verbose test -z "$__gitcomp_builtin_notes_edit"
> +'
> +
>  test_done
> --
> 2.17.0.rc0.103.gbdc5836ed3
>



-- 
Duy