From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id 23bb443d for ; Fri, 29 Nov 2019 20:31:37 +0000 (UTC) Received: (qmail 1435 invoked by alias); 29 Nov 2019 20:31:30 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 44956 Received: (qmail 23352 invoked by uid 1010); 29 Nov 2019 20:31:30 -0000 X-Qmail-Scanner-Diagnostics: from out5-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.0/25642. spamassassin: 3.4.2. Clear:RC:0(66.111.4.29):SA:0(-2.6/5.0):. Processed in 1.600762 secs); 29 Nov 2019 20:31:30 -0000 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= daniel.shahaf.name; h=date:from:to:cc:subject:message-id :references:mime-version:content-type:content-transfer-encoding :in-reply-to; s=fm1; bh=+BHNxVxQ2KfFMCc+Y24DG60NGXCfh1550oQQ0Gdy vIY=; b=bAvpesBh0/FPJH686LpPGsC8YHAgpZukJzqVD5Rfnv7v7L1X4TN39eXX Jmvd1EB/LRHnS3hATD8PENRTmBfvQ5aF9YNltpYnI3BTZX7QvmgY3/cVvQ7teydQ Meab5jneScebS+58YI64hVMty5cDYV/yW+AIhbdkEnZTbp0L7qW5rq5sahHBXCuu me2RLjVP+q6+dyzQoIyXTiABlesMwdrKpFPKZA4PRHSWiXrMVl7b3cU9wwbtncRz tpO+Kd8av+tTFtQdzMewPd0pxaSMb2Sz/omVBxVGDo4G2QxfxTrbCkCNNtBimJp0 GTEAfo4Npv5o8LHkLipLGRcxqr9c/A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=+BHNxVxQ2KfFMCc+Y24DG60NGXCfh1550oQQ0Gdyv IY=; b=QeJunegsBvCtNSIuFVt4ErOkxwqxZ5ZTBx9ZmOpZ6e94npY1TwvBMz6YR VR9gpNkJNi8d+jlXzcexkdWa1Hjw6gPxe/MmgOPigeJA6hL2a1Qmk/5cPxcjw7c1 x7zKozn7/EfEMzQTCFMf7iCMr4qMMEWIF2GBRFHQa3tSpvpkiUuPF2mebRl4wBcq VCDv1fXTldgNZbFD8IemFoPPCRKJZbaXO1WkYezlgi/9UWZMoXHxkaAFmVrnHeth 4dMA7rjxHL7rPAC6qnbTa7/83Y7J5yjBC90UsTfNWGaJvyPQuU1n0LvgQzImN5pn Kqyg4BejVZtNNkIrDX+5B+SrPlF9g== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrudeiledgudefgecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvuffkfhggtggugfgjfgesthektddttderjeenucfhrhhomhepffgr nhhivghlucfuhhgrhhgrfhcuoegurdhssegurghnihgvlhdrshhhrghhrghfrdhnrghmvg eqnecukfhppeejledrudektddrheejrdduudelnecurfgrrhgrmhepmhgrihhlfhhrohhm pegurdhssegurghnihgvlhdrshhhrghhrghfrdhnrghmvgenucevlhhushhtvghrufhiii gvpedt X-ME-Proxy: Date: Fri, 29 Nov 2019 20:30:52 +0000 From: Daniel Shahaf To: Aleksandr Mezin Cc: zsh-workers@zsh.org Subject: Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs Message-ID: <20191129203052.fpzwna5jv5i5tqu3@tarpaulin.shahaf.local2> References: <20191125043525.lcxm532gi6hb7n53@tarpaulin.shahaf.local2> <9438ca11-d0be-4c67-a5b3-a0b900342302@www.fastmail.com> <5edaa985-4ac9-411c-b4c2-415c49a563e3@www.fastmail.com> <20191128213430.53b5akiq43l7bzbx@tarpaulin.shahaf.local2> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Aleksandr Mezin wrote on Fri, Nov 29, 2019 at 19:41:51 +0600: > On Fri, Nov 29, 2019 at 3:34 AM Daniel Shahaf wrote: > > > > Daniel Shahaf wrote on Thu, Nov 28, 2019 at 11:13:44 +0000: > > > Aleksandr Mezin wrote on Tue, 26 Nov 2019 10:22 +00:00: > > > > On Tue, Nov 26, 2019 at 10:52 AM Daniel Shahaf wrote: > > > > > % cd foo > > > > > [shows hg info] > > > > > % export GIT_WORK_TREE=${PWD/foo/bar} GIT_DIR=${PWD/foo/bar}/.git > > > > > [shows git info] > > > > > % > > > > > > > > Interesting. I didn't notice that GIT_INFO_detect_git checks the exit > > > > code of `git rev-parse --is-inside-work-tree`, not the actual value. > > > > > > I think it should check both the exit code and the output. > > > > Sorry, I didn't look closely enough. In the example I gave (quoted above), > > --is-inside-work-tree prints "false" and exits with code 0. In that situation, > > I think VCS_INFO_detect_git should detect git, and it does. All in all, > > I think the current code is correct [...] > > Then I don't see a sane way to choose which VCS to show, other than > how it's already done (first one detected from 'enabled'). For > example, a hg work tree in cwd, and GIT_WORK_TREE is set to something > else (maybe parent directory, maybe ../other-dir). Which one (hg or > git) to choose? Why? As I said in workers/44930 and workers/44936, I propose to show information for the VCS whose worktree root is deeper; and if neither worktree root is deeper than the other, _only then_ fall back to the current behaviour of showing information for the VCS listed first in the 'enable' style. I think this would address your use-case without regressing any other. (By "worktree root" I mean the directory shown by «git rev-parse --show-toplevel», «svn info --show-item=wcroot», and their equivalents for other VCS's.) With this proposal, there are three cases to consider: ${basedirA:P} and ${basedirB:P} may be either (a) equal, (b) siblings, or (c) parent and child. The proposal makes no difference for cases (a) and (b). For case (c), it allows the user to see info for either the parent vcs or the child vcs by cd'ing appropriately. That would be an improvement over the current behaviour. Yes, in cases (a) and (b) there is no obvious correct behaviour. However, the fact that there's no obvious correct behaviour in a given situation is simply a data point that should inform the design process; nothing more, nothing less. It doesn't make the problem unsolvable. > I guess I'll have to live with my own fork of vcs_info, because any > changes there will break it for someone else. Let's not jump to conclusions. In this case, I think you have several courses of action open to you: - Revise your patch series in light of the feedback received — for example, workers/44927 proposed two small changes to the «if (( ${#vcs_comm[basedir]:a} > ${#nearest_vcs_comm[basedir]:a} ))» line — and resubmit it. [The changes are basically what I wrote in workers/44936, taking the bulleted list for an if/elif/else chain. I think there already is consensus on those changes.] - Propose some other API change; for example: + Make it possible to report on multiple VCS's at the same time (cf last paragraph of workers/44936). This will cover not only the parent/child case but also the "same" and "sibling" cases. I would recommend to do this by writing up a behaviour specification [ideally as a _pro forma_ patch to Doc/Zsh/contrib.yo] and putting it up for discussion, rather than by implementing anything. + Add a hook that gets the list of VCS's that have been recognized and sets $REPLY to the name of the VCS backend to show info for. + Document that in the the "same" and "sibling" cases, it is unspecified which VCS's info gets displayed — i.e., vcs_info will show the info for one of the backends, but we don't promise which one — and recommend to set styles as Mikael explained. + Something else. The (( $+GIT_WORK_TREE )) && [[ -e ./.hg ]] case is an edge case. Since it doesn't work _today_, fixing it isn't a blocker to accepting the patch. It's perfectly fine to fix just the parent/child case and leave the "same" and "sibling" cases as they are today. (Patches should introduce no regressions, but aren't expected to fix all existing bugs.) In the meantime, the first two patches in this series can be applied, can't they? Or were you planning to revise and resubmit the CVS one to address the (preëxisting) infinite loop? The first two patches should cause no behaviour change, I assume, other than making $hook_com[basedir] available to hooks earlier. Actually, should we document $hook_com[basedir] in the manual? Cheers, Daniel P.S. I don't know what your background is, but in general, it's _very_ common for patch series to receive reviews and be resubmitted with changes before they're accepted and merged; that's an everyday occurrence, no more remarkable than seeing somebody give up their seat on a city bus.