From: Daniel Shahaf <email@example.com> To: Aleksandr Mezin <firstname.lastname@example.org> Cc: email@example.com Subject: Re: [PATCH v5] vcs_info: choose backend by basedir Date: Fri, 26 Nov 2021 08:08:38 +0000 [thread overview] Message-ID: <20211126080838.GA17935@tarpaulin.shahaf.local2> (raw) In-Reply-To: <firstname.lastname@example.org> Sorry for how long this is taking. I've not had much time recently. (I haven't even had time to commit all my own patches.) I think the summary as of right now (before this post) is still workers/48329. (I did another review after that, but answers to it have been incorporated into v5 here.) -workers@: given that several people have mentioned they weren't familiar with vcs_info enough to review/apply this, I'd like to particularly solicit reviews of the new functionality, as described in the log message and docs patch. I'm not looking for review of the implementation or for copyediting of the docs, but for reviews of the functionality described in the docs. Before I get to the details, I'd like to repeat that I'm aware of how old this patch is — it's two years old this week — and I'll adjust my review granularity accordingly, as far as reasonable without cutting corners on my committability standards. Aleksandr Mezin wrote on Fri, Jun 04, 2021 at 12:14:21 +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. > We seem to have consensus to add such a functionality, established in workers/48329 (grep for "First things first"). > 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. > Okay, so the order is actually: 1. Backends that returned a non-ancestor directory 2. First detected backend out of those that don't have choose-closest-backend set 3. Closest ancestor backend out of those that do have choose-closest-backend set Now, I'm sure that achieves your original design goal (from 44918) of preferring hg to git inside ~/git-repo/hg-repo, but overall it seems rather arbitrary. What's the rationale for this particular order? How does one remember what has precedence over what? (E.g., command-line options generally have precedence over environment variables because command-line options are assumed to have been set "closer" to running the command.) My question from 48329 stands: . 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? . To be clear, the question is why $GIT_WORK_TREE should have precedence over VCS_INFO_detect_fossil() regardless of style settings. I think we could simplify this design by dropping the ability to set the style on a per-backend basis. Let there be two modes: the current one (of detected backends, choose whichever comes first in the 'enable' style's value) and the new one (of detected backends, choose the one whose worktree root is deeper), with a global knob to choose between them. That should suffice. Furthermore, why doesn't the first mode suffice on its own? That is: for your ~/git-repo/hg-repo situation, why is it not sufficient to ensure that hg precedes git in the value of the 'enable' style? I couldn't find the answer in the past threads. > 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. Well, I suppose one might set: . zstyle ':vcs_info:*' choose-closest-backend yes zstyle ':vcs_info:git:*' choose-closest-backend no . The effect of the second line would be to give git precedence when it's the outer VCS. I don't immediately see a use-case for this. Perhaps if someone has several sibling worktrees, and they then create an outer worktree encompassing all inner worktrees so as to make/revert changes in lockstep across all trees? That's relevant if the outer worktree isn't of the same VCS as the inner ones. > Signed-off-by: Aleksandr Mezin <email@example.com> > --- > v4: back to boolean zstyle, keep old behavior by default, documentation > v5: described expected vcs_info[basedir] use in a comment, addressed Daniel's > questions in comments. Hope this helps. I'm sure it feels like we're going backwards, or in circles, but one always feels a jolt backwards when a car starts moving ☺ Cheers, Daniel
next prev parent reply other threads:[~2021-11-26 8:09 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-04 6:14 Aleksandr Mezin 2021-06-20 18:52 ` Lawrence Velázquez 2021-07-18 18:42 ` Lawrence Velázquez 2021-07-28 18:00 ` Bart Schaefer 2021-11-23 16:03 ` Aleksandr Mezin 2021-11-26 8:08 ` Daniel Shahaf [this message] 2021-11-26 8:09 ` RFC: Remove vcs_info backends cdv, svk, tla? (was: Re: [PATCH v5] vcs_info: choose backend by basedir) Daniel Shahaf 2021-11-26 9:43 ` Oliver Kiddle 2022-02-20 8:49 ` [PATCH] vcs_info, run-help: Remove Codeville (cdv), svk, GNU arch (tla) Daniel Shahaf
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211126080838.GA17935@tarpaulin.shahaf.local2 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH v5] vcs_info: choose backend by basedir' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).