From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 16599 invoked from network); 17 Apr 2021 12:18:25 -0000 Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368) by inbox.vuxu.org with ESMTPUTF8; 17 Apr 2021 12:18:25 -0000 ARC-Seal: i=1; cv=none; a=rsa-sha256; d=zsh.org; s=rsa-20200801; t=1618661905; b=RcOgVuxuWSsTJ8h2ym8AvXYAiYrbr7bhfc9idBF6j6Nhk0mAaP7M5ShSJhklcoN0ym5qiNQWqh W6dlVw6Emp+dVKA35K8RPG9S+9lCS+D3lI+rN4yedH3RUuWgszGzJiH3Jsb7tevZ9fNiX6VGtb /EsXljLysYi8NVBU617MRE8tK4jqDvnTeHMsHCsxwlxx+l5hOjzIlbFsyD3qCdTzcn3af3ggiM CZ/698u2/HDB+NyJxOavCJK2MKK0GKhlf+Sd4fP9o2pK77juuctajTKWKi2Wla3KB7cSb0++gK o1SO/6mHuoyM8KRYCg1OYShRqr7GPRI5LQ4qbsNRMr9few==; ARC-Authentication-Results: i=1; zsh.org; iprev=pass (mail-lf1-f52.google.com) smtp.remote-ip=209.85.167.52; dkim=pass header.d=gmail.com header.s=20161025 header.a=rsa-sha256; dmarc=pass header.from=gmail.com; arc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed; d=zsh.org; s=rsa-20200801; t=1618661905; bh=H4H1wFNpWFZ6C6HeoBcluk2LF7der2OYdCnx7kzwVCU=; h=List-Archive:List-Owner:List-Post:List-Unsubscribe:List-Subscribe:List-Help: List-Id:Sender:Content-Transfer-Encoding:Content-Type:Cc:To:Subject: Message-ID:Date:From:In-Reply-To:References:MIME-Version:DKIM-Signature: DKIM-Signature; b=l2utSgozKWFrRl31On4r8lBi7FmsGfWqs/b4wxR9lX90XZ4zGencWEHqBdK2nTv8AIXXZdYxr3 dPrMr6y9NOkXmTnh0beAXuTjfg5LaB8MmPUQagIpf8hsi58q7jfmQ7uboUTAcG4bQ49yLJtvh1 PMuV9aPClHsGth8Rl/U4H+VZi6rxQT1AegEOQl+rBG1b5eDZj+SlBtEsUT72Crq5/sXlLR+6o8 m9lZTBgrzqjIImFeaBgyyKIvCzvKtEHFVuaq5Ni93ajOH5lslsXmSeLYzRsk27FsQeZLch8Nbu PAbPXshW8AScwhG8hkf23bdp5jwcYGF0v4bINbwAizaOLg==; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=zsh.org; s=rsa-20200801; h=List-Archive:List-Owner:List-Post:List-Unsubscribe: List-Subscribe:List-Help:List-Id:Sender:Content-Transfer-Encoding: Content-Type:Cc:To:Subject:Message-ID:Date:From:In-Reply-To:References: MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=sARjQ/maA/8/SXgIGIwb204ORWw9MnFEj6dA2vIdxvM=; b=lkNTfOoh86zCPFVQum41leykQc wBGg0zHJlKrAnYlnUdk2+wnOUs1GCbLAHvZo27i4hARuN8p57UkELS3LDvu3UVCg+kTFbBYnI9ru9 9B1dn3Ylhk3E8xUlg9JU5QWlSLqyWCQlBIHqAN4QN+qF7M56JVHso5AHcsPFPkC2I+uiWBXqrELdS m7+GIYVCEqTHnbJqdpcsZrBUmEoh/gpdy0DqgW4a0IYtPINYk3tLegfI1Y887IAbLMgYklqE/O2f4 1x+aXEp/Ih2icxLZiCRQnwlNc1QsuJdwQ9L//CETxgOhizI2a2ADcGBaPAw7hmTea1bcIkSGK0l8e xGkd2Q0Q==; Received: from authenticated user by zero.zsh.org with local id 1lXju0-000JCu-Oa; Sat, 17 Apr 2021 12:18:20 +0000 Authentication-Results: zsh.org; iprev=pass (mail-lf1-f52.google.com) smtp.remote-ip=209.85.167.52; dkim=pass header.d=gmail.com header.s=20161025 header.a=rsa-sha256; dmarc=pass header.from=gmail.com; arc=none Received: from mail-lf1-f52.google.com ([209.85.167.52]:44576) by zero.zsh.org with esmtps (TLS1.3:TLS_AES_128_GCM_SHA256:128) id 1lXjtE-000Iy0-Gf; Sat, 17 Apr 2021 12:17:33 +0000 Received: by mail-lf1-f52.google.com with SMTP id i10so12984563lfe.11 for ; Sat, 17 Apr 2021 05:17:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=sARjQ/maA/8/SXgIGIwb204ORWw9MnFEj6dA2vIdxvM=; b=qTmTeuBZEEcrlcp5qQZxCxFWrNDKYBUPEqCO4FTBDYSJUk9yWwF7tN+Dx3KyQSn/k+ 7p6WV+j7QpUmpCuQ8QyrhMMrSCKw+il3ras884MI27xBYI0gVsm+PEEyhxqfNaGETkra uTdmmFP74bNsFDzeNhgVaj/69YzEfejJjlLWqNUq6FciLAGWJ2xN81xjkCxz9IKNAiw1 aUciObuKNBZ4bJm4m0bQaiQkd44h0adwh2nkWtW4w4qIZJmreYHywBvJ+KBjmG8jMfX+ OW7Hi9PIFFNSvZnvmlJ2+UAN7FgPla+L7MyCPu5uDMZRqIZO7/+qvYWVofwtOhJ9+nIo o5FQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=sARjQ/maA/8/SXgIGIwb204ORWw9MnFEj6dA2vIdxvM=; b=bc8dk4v++Ccj9xS9bLfJbtomFegBL+GG2N0LbiiRNlzpEC/8Hb1sD2aaFCXpL012Wr 3Uzqu5qboK6Eg+o5wTOdxqiWTvhUXSdPDca4vnOoL8uEuip6Z/+drjXS9F3GLgBxCSrB noiz4ZwayQIUqwW9MqFDUPAMDjmg3Q+8nDnP4IQgalgPrxXkjguQckbZZ5ZyLGeqaiR+ 1ElGbQXt07blCIkr/mXPjW1qXwsQYitKLScH14+yV1uhOosf/zsyj5o0Fa3m/SrmNXI0 /MRzdnmB2qhNRH1IOChZ3ZRKj1PWGtmnRcyw34wHHZRG/uGx9PURVG7J777eE4kyfFE0 FexQ== X-Gm-Message-State: AOAM533YAnJN3MLkUQUwDda2XExXSjjwhcBFs3xri7HTYeRoUvDbDHsq h38nYSIdu4OURxnePUWcFk9dTf3RZoNKiiwstwCFWOCp24k= X-Google-Smtp-Source: ABdhPJw3TZzMn/9G3hd0sCtsgBHArxpUXvSQHfoCd0DXivq16zqcApIVxIxibOPTsu86MfcDV6FgA3QuKjU0C3497/8= X-Received: by 2002:ac2:5fab:: with SMTP id s11mr5924868lfe.401.1618661851597; Sat, 17 Apr 2021 05:17:31 -0700 (PDT) MIME-Version: 1.0 References: <20201114193703.1428617-1-mezin.alexander@gmail.com> <20210329165629.GA2761@tarpaulin.shahaf.local2> In-Reply-To: <20210329165629.GA2761@tarpaulin.shahaf.local2> From: Aleksandr Mezin Date: Sat, 17 Apr 2021 18:17:20 +0600 Message-ID: Subject: Re: [PATCH v4] vcs_info: choose backend by basedir To: Daniel Shahaf Cc: zsh-workers@zsh.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Seq: 48610 Archived-At: X-Loop: zsh-workers@zsh.org Errors-To: zsh-workers-owner@zsh.org Precedence: list Precedence: bulk Sender: zsh-workers-request@zsh.org X-no-archive: yes List-Id: List-Help: List-Subscribe: List-Unsubscribe: List-Post: List-Owner: List-Archive: On Mon, Mar 29, 2021 at 10:56 PM Daniel Shahaf wro= te: > > Let me first summarize the previous threads: > > - The current default is that backends are used in order of the =C2=ABena= ble=C2=BB > 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 repo= sitory > > status from the first backend that works (i.e. first backend that can f= ind a > > repository). > > > > But I prefer a different behavior: I want to see the status of the repo= sitory > > closest to the current working directory. For example: if there is a Me= rcurial > > repository inside a Git repository, and I 'cd' into the Mercurial repos= itory, > > 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 direct= ory, > > old algorithm is used: whichever comes first in 'enable' list. > > > > 2. If all backends return basedirs that are ancestors of the current di= rectory, > > 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_TR= EE), > > 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? Can it be addressed later in a separate patch (if necessary at all)? > > Should we implement, say, a hook that gets arguments of the form > =C2=AB${vcs}\0${worktree_root}=C2=BB 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 thoug= h. > > > > Signed-off-by: Aleksandr Mezin > > --- > > 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=3D() > > VCS_INFO_get_cmd > > - VCS_INFO_detect_${vcs} && (( found =3D 1 )) && break > > + if VCS_INFO_detect_${vcs}; then > > + basedir_realpath=3D"${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) I don't get what exactly needs to be documented (and where). vcs_comm doesn't seem to be documented anywhere (except the comment "vcs_comm is used internally for passing values among VCS_INFO_* functions. It is not part of the public API."). > > > + zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" choose-= closest-backend > > + choose_first_available=3D$? > > + > > + [[ "${PWD:P}/" =3D=3D "${basedir_realpath}"/* ]] || choose= _first_available=3D1 > > If I'm not mistaken, this condition will false negative when > [[ $basedir_realpath =3D=3D / ]] and [[ $PWD !=3D / ]]. Yes. Should probably be `[[ "${PWD:P}/" =3D=3D "${basedir_realpath}"/* ]] || [[ "${basedir_realpath}" =3D=3D "/" ]] || choose_first_available=3D1`. Should I send v5 now? > > > + if (( choose_first_available )) || (( ${#basedir_realpath}= > ${#chosen_basedir_realpath} )) ; then > > + chosen_vcs=3D"${vcs}" > > + chosen_vcs_comm=3D("${(kv)vcs_comm[@]}") > > + chosen_backend_misc=3D("${(kv)backend_misc[@]}") > > + chosen_basedir_realpath=3D"${basedir_realpath}" > > + > > + (( choose_first_available )) && break > > + fi > > + fi > > done > > > > - (( found =3D=3D 0 )) && { > > + vcs=3D"${chosen_vcs}" > > + vcs_comm=3D("${(kv)chosen_vcs_comm[@]}") > > + backend_misc=3D("${(kv)chosen_backend_misc[@]}")