Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
Re: [PATCH v4] vcs_info: choose backend by basedir
- X-seq: zsh-workers 48329
- From: Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>
- To: Aleksandr Mezin <mezin.alexander@xxxxxxxxx>
- Cc: zsh-workers@xxxxxxx
- Subject: Re: [PATCH v4] vcs_info: choose backend by basedir
- Date: Mon, 29 Mar 2021 16:56:29 +0000
- Archived-at: <https://zsh.org/workers/48329>
- Archived-at: <http://www.zsh.org/sympa/arcsearch_id/zsh-workers/2021-03/20210329165629.GA2761%40tarpaulin.shahaf.local2>
- In-reply-to: <20201114193703.1428617-1-mezin.alexander@gmail.com>
- List-id: <zsh-workers.zsh.org>
- References: <20201114193703.1428617-1-mezin.alexander@gmail.com>
Let me first summarize the previous threads:
- The current default is that backends are used in order of the «enable»
style: first detected, wins.
- Frank prefers this remain the default. (workers/47488, workers/47509)
- Aleksandr implemented a mode in which the backend whose worktree root
directory is closest to $PWD would be used.
- Bart's use-case (workers/44931) wouldn't be served by choosing the
"deepest" worktree but by choosing whichever one has local mods.
(workers/47490)
- It was proposed to address Bart's use-case by calling vcs_info twice
with different user-context strings and corresponding zstyle settings.
- Oliver proposed some independent improvements in workers/47492.
If any of that is inaccurate, speak up. The named people are Bcc'd.
The patch being replied to contained some git- and cvs-specific changes
which have been committed. Aleksandr expressed concerns about those in
workers/44929, but as far as I can see, the part that has been committed
is a no-op, other than the fact that vcs_comm[basedir] will be populated
earlier.
So, first things first: Does implementing a "choose the VCS whose root
is deepest" mode that's off by default sound like a reasonable way
forward? Any concerns with that? Any concerns with the algorithm
described in Aleksandr's log message (just the log message,
independently of the unidiff)?
Below a couple of review points on my account. Not a full review; just
what I spotted from reading the diff.
Aleksandr Mezin wrote on Sun, Nov 15, 2020 at 01:37:03 +0600:
> Previously, vcs_info iterated over enabled VCS backends and output repository
> status from the first backend that works (i.e. first backend that can find a
> repository).
>
> But I prefer a different behavior: I want to see the status of the repository
> closest to the current working directory. For example: if there is a Mercurial
> repository inside a Git repository, and I 'cd' into the Mercurial repository,
> I want to see the status of Mercurial repo, even if 'git' comes before 'hg' in
> the 'enable' list.
>
> This patch adds a new algorithm for vcs_info to choose the backend:
>
> 1. Among backends whose basedir isn't an ancestor of the current directory,
> old algorithm is used: whichever comes first in 'enable' list.
>
> 2. If all backends return basedirs that are ancestors of the current directory,
> the one with basedir closest to the current directory is chosen.
>
> As far as I know, unless the user has set basedir manually (GIT_WORK_TREE),
> all backend return basedirs that are ancestors of the current directory.
> Backends that return non-ancestor basedirs have higher priority to show the
> status of that manually-configured repository.
>
Do any other backends allow the worktree to be somewhere other than an
ancestor of $PWD? Git can, as mentioned above. Subversion can't.
(svn(1) always operates on the path given by the positional arguments,
if any, else on $PWD.) I'm unsure about others.
There doesn't seem to be any way to shadow a GIT_WORK_TREE setting,
other than to temporarily set the enable/disable styles so as to exclude
git entirely. Is that a problem?
Should we implement, say, a hook that gets arguments of the form
«${vcs}\0${worktree_root}» and sets $reply to the one to use? We could
pass the arguments sorted by disk path and by order of the 'enable'
style. I suspect this would be overengineering.
> The algorithm used by vcs_info can be configured using 'choose-closest-backend'
> zstyle:
>
> zstyle ':vcs_info:*' choose-closest-backend yes
>
> By default, old algorithm is used.
>
> It can also be configured per-backend:
>
> zstyle ':vcs_info:hg:*' choose-closest-backend no
>
> This effectively moves the backend into group (1), as if its basedir is never
> an ancestor of the current directory. Not sure how useful this is though.
>
> Signed-off-by: Aleksandr Mezin <mezin.alexander@xxxxxxxxx>
> ---
> v4: back to boolean zstyle, keep old behavior by default, documentation
>
> Completion/Zsh/Command/_zstyle | 1 +
> Doc/Zsh/contrib.yo | 41 +++++++++++++++++++
> Functions/VCS_Info/vcs_info | 30 ++++++++++++--
>
> +++ b/Functions/VCS_Info/vcs_info
> @@ -124,10 +126,30 @@ vcs_info () {
> fi
> vcs_comm=()
> VCS_INFO_get_cmd
> - VCS_INFO_detect_${vcs} && (( found = 1 )) && break
> + if VCS_INFO_detect_${vcs}; then
> + basedir_realpath="${vcs_comm[basedir]:P}"
If vcs_comm[basedir] starts to be used like that, it should be
documented as an internal API. (In comments, not yodl)
> + zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" choose-closest-backend
> + choose_first_available=$?
> +
> + [[ "${PWD:P}/" == "${basedir_realpath}"/* ]] || choose_first_available=1
If I'm not mistaken, this condition will false negative when
[[ $basedir_realpath == / ]] and [[ $PWD != / ]].
> + if (( choose_first_available )) || (( ${#basedir_realpath} > ${#chosen_basedir_realpath} )) ; then
> + chosen_vcs="${vcs}"
> + chosen_vcs_comm=("${(kv)vcs_comm[@]}")
> + chosen_backend_misc=("${(kv)backend_misc[@]}")
> + chosen_basedir_realpath="${basedir_realpath}"
> +
> + (( choose_first_available )) && break
> + fi
> + fi
> done
>
> - (( found == 0 )) && {
> + vcs="${chosen_vcs}"
> + vcs_comm=("${(kv)chosen_vcs_comm[@]}")
> + backend_misc=("${(kv)chosen_backend_misc[@]}")
Messages sorted by:
Reverse Date,
Date,
Thread,
Author