From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27264 invoked by alias); 13 Nov 2015 19:22: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: X-Seq: 37104 Received: (qmail 29126 invoked from network); 13 Nov 2015 19:22:26 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.0 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= daniel.shahaf.name; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=0J6V8CGaq7RRc0ffaDAaofsie7c=; b=OCTDtg nXYzk9nCP9FgAIh3QfXLaI63nSiWogVfHeEOBd8/koj2K238ZaT69zqZHvFSYSlI Uyd9LoGTLqKx6Mpn7IW0IjGOC9KP9PDoUxlA52vsMcGT9aoQgAt+g0K5xIaxxN/F CYtrkTNKO2iHQCrHDXQmKHTv8dpT/5ZWRDgZQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=smtpout; bh=0J6V8CGaq7RRc0ffaDAaofsie7c=; b=aV3LL bJxzKDBC/HtwfXBa0EjrV+BKXFaRtrIvka1fPVTNyQiFrigAwMapO+KMYLzd7ocm MLl2BVeTY5YQ0MB5aJE14hig6jl1xoDpr9z+D9AWyW8yQbsVflvNCzfPBkW2qDR8 ez1ILLfVctmvvEC2TI2Q1jlpp2WR/qw0+WqV5Y= X-Sasl-enc: S16AmUze/KxF55VaE6f+vBEkGVc7H7z2S6LlIURXkAbZ 1447441887 Date: Fri, 13 Nov 2015 19:11:21 +0000 From: Daniel Shahaf To: Oliver Kiddle Cc: zsh-workers@zsh.org Subject: Re: __git_recent_commits cannot be called twice Re: [PATCH 2/5] _git: Offer @~$n as completion of recent commits. Message-ID: <20151113191121.GA2390@tarsus.local2> References: <20151025183458.GK11372@tarsus.local2> <20151031125127.GA2360@tarsus.local2> <25552.1446323081@thecus.kiddle.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <25552.1446323081@thecus.kiddle.eu> User-Agent: Mutt/1.5.21 (2010-09-15) Oliver Kiddle wrote on Sat, Oct 31, 2015 at 21:24:41 +0100: > Daniel Shahaf wrote: > > > The new output works fine in 'git commit --fixup=', but not in 'git > > show '. This is because the latter calls __git_recent_commits via > > two distinct codepaths. Here's a minimal example: > > Calling one function via two codepaths is the basic problem here. We > should avoid doing that rather than trying to hack it to work. > > > 25 HEAD master > > 26 @~0 -- [HEAD] m (71 seconds ago) > > 27 5d97266 -- [HEAD] m (71 seconds ago) > > > > This is only a workaround because grouping @~0 and 5d97266 is desirable, > > since they both refer to the same commit. > > Is it really useful to add matches for the abbreviated hashes such as > 5d97266? I never type them. Matches for the HEAD form might be more I would use the @ syntax when trying to complete "the penultimate commit" (when I don't even know its hash), but I would type a hash directly when trying to complete a commit from 'git log' output by typing its first few hex digits. > useful. I'd also consider using the prefix-needed style with the @ > form and perhaps even with HEAD. It might be better to cope with the I think it depends on context: prefix-needed would make sense for recent commits in context of 'log' or 'diff' but not for 'commit --fixup='. (Incidentally, the latter is also an example of a second way that hashes are useful: 'git commit --fixup=7f' is an effective way of picking an arbitrary commit from the list shown by the first .) > problem of more than one commit having the same comment by not using > _describe but adding matches manually. > > > The underlying problematic behaviour is reproducible without _git: > > > > 1 $ zsh -f > > 2 % _f() { local -a x=(foo:aaa bar:aaa); repeat 2 _describe -tt 'desc' x } > > 3 % compdef _f f > > _describe removes the 'rows' token from compstate[list]. So given the (compstate[list] and LIST_ROWS_FIRST being related) > following set of matches/descriptions: > w x -- 0 > y z -- 1 > It adds matches in column first order: > w y x z 0 1 > > This means the descriptions are all added last so in this case: > compadd -E2 '-- 0' '-- 1' > > With two separate _describe calls, it is like adding the matches in this > order (with setopt nolistrowsfirst): > w x 0 y z 1 > The description matches are very wide due to space padding so it is no > longer able to arrange matches in columns - hence the output you > described. > That makes sense. I've been able to produce the buggy output even when there's just one pair of matches, though: for example, 'f' in the following example produces the buggy output when LIST_PACKED is unset. Presumably that's just a bug in the width calculation code (uniquification not happening early enough?). # to be run in an 80-column terminal # (else change the "10 hours ago" string" to have width $((COLUMNS - 16))) autoload -Uz compinit compinit _f() { typeset -a _tmpd _tmpd=( 5d97266 ) typeset -a _tmpm _tmpm=( 5d97266 ) compadd -2V commits -X '%B> %Urecent commit object name%u%b' -d _tmpd -a _tmpm typeset -a _tmpd _tmpd=( '@~0' ) typeset -a _tmpm _tmpm=( '@~0' ) compadd -2V commits -X '%B> %Urecent commit object name%u%b' -d _tmpd -a _tmpm typeset -a _tmpd _tmpd=( '- [HEAD] m (10 hours ago) ' ) typeset -a _tmpm _tmpm=( ) compadd -E1 -V commits -X '%B> %Urecent commit object name%u%b' -d _tmpd -a _tmpm } _g() { _f; _f } compdef _f f; compdef _g g; # optionally setopt listrowsfirst and/or listpacked (That 'f' is just the guts of _describe, that's why the strange _tmpx variables.) > Perhaps you could change compdescribe to do things in row first order > but it may have other associated problems, particularly when it comes to > the algorithm used to pack matches in suitable column widths. > I probably could, if only I had access to the secret repository containing the source code comments for the completion code ;-) I don't think I understand the completion code (_describe, compdescribe, compadd) well enough to decide what change to make and how to make it. > _describe was written with the assumption that it is fully responsible > for the match group it is adding matches for. It is best to only use it > when that is the case. Isn't that what __git_recent_commits is doing? _describe is solely responsible for the "hashes and @~N" portion of the output. > All these implementation considerations aside, the output I'm currently aiming for is: 5d97266 @~0 - [HEAD] m (10 hours ago) 3edcf9b @~1 - [HEAD~1] m (11 hours ago) that is: one line per commit, two matches per line. But I'm open to be convinced that some other output would be better. Thanks, Daniel P.S. I'm sorry for having taken so long to reply. If I'd known I wouldn't be able to reply for so long, I wouldn't have sent the email when I did. (I'd have waited to send the email until such a time when I was able to reply timely.)