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,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 23812 invoked from network); 20 Mar 2021 20:54:37 -0000 Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368) by inbox.vuxu.org with ESMTPUTF8; 20 Mar 2021 20:54:37 -0000 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:Message-ID:Date: Content-Transfer-Encoding:Content-ID:Content-Type:MIME-Version:Subject:To: References:From:In-reply-to:Reply-To:Cc:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=xyKs+n19CT4CmwL7VUGyCVAN2p19e1ZC1wdIxfdRJRY=; b=b/VIVZlhwFhksNpd16gnKWM6Da U2O50fjnNGrX2FJJ5aIO3ooxdB35keUSCMF9qrDn3d/LqhOIfsY6IgkqMxZF2z9kCF6s3zguMWSB1 aAXSkoavN1E4YLiBhkYVyZM7uXP2QS0SHIbByOxL2SnSc/nfMJzJ3O/pwOrmFsiqZs22P3xWMED07 ipDDJ6BU1sgneOfD4Ek4vm+9oU47Gc3JfEbK2xkYWsEtBspQ7NCf6jJNH7Koienwr2UU1PsHyFCCK YwT634er14f742SjRi8tIfKvw80QKtkA2Szbs2gsXDXqil+Mh1JqNteKL5xAYPF/F3jsdOxlsKBm3 wIekRUYQ==; Received: from authenticated user by zero.zsh.org with local id 1lNicH-00014y-8c; Sat, 20 Mar 2021 20:54:37 +0000 Received: from authenticated user by zero.zsh.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) id 1lNiby-0000uv-W5; Sat, 20 Mar 2021 20:54:19 +0000 Received: from [192.168.178.21] (helo=hydra) by mail.kiddle.eu with esmtp(Exim 4.93.0.4) (envelope-from ) id 1lNibv-000G31-2u; Sat, 20 Mar 2021 21:54:17 +0100 In-reply-to: From: Oliver Kiddle References: <6c372e58-7709-4629-b2d0-e472a120404d@www.fastmail.com> To: "Marlon Richert" , zsh-workers@zsh.org Subject: Re: Bug + patch: _expand() fails to insert unambiguous prefix MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-ID: <61689.1616273655.1@hydra> Content-Transfer-Encoding: 8bit Date: Sat, 20 Mar 2021 21:54:15 +0100 Message-ID: <61690-1616273655.087174@mxKg._JXT.SUbL> X-Seq: 48205 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: Archived-At: Lawrence Velázquez wrote: > Sorry, that was more to elicit a review from a dev, but you're welcome to expand the patch of course. _expand takes a deliberate approach of setting compstate[insert]=menu That forces menu completion and was a conscious design decision. This tends to be applicable for completions that aren't taking an incomplete prefix and adding characters but are transforming what was entered. It is reasonable that you might prefer different behaviour here so I'm not averse to enabling it via a style. A simple pattern like a plain * makes for a rather contrived example. With a more realistic pattern, the case is less clear. I've always used just the all-expansions tag with _expand. I don't see why you'd want completion to turn a carefully written pattern into two characters of common prefix. The trouble is that this patch has needed to change _main_complete, so it results in a change in the behaviour of any other completion function that sets compstate[insert]. In quite a few cases, _pids for instance, I don't think the change is an improvement. The relevant part of _main_complete checks whether compstate[insert] has been changed by the function and applies the values from styles and defaults if not. The patch continues regardless of the compstate[insert] change. In general, it is good if the user can override defaults but this is changing the default behaviour. The menu style does get looked up with the context :completion::expand:::expansions but _expand setting compstate[insert] overrides it. Ideally, the precedence order for preferences should be: 1 User style configured for a specific context (:completion:*:expand:*:expansions) 2 Specific default set explicitly by a completion function, _expand in this case 3 User style configured for the general context (menu style default tag in this case) 4 A general default Your patch is throwing away (2) in a limited manner. The old code gave (2) precedence over (1). Getting it right is not simple, though - consider also the fact that the menu style is not really meant to control the unambiguous aspect of compstate[insert]. But I think we can do better while preserving existing behaviour. For what it's worth, I'm also not keen on the patch leaving a block with the wrong indentation level (apart from a final fi and preceding line). It kept the patch short but makes it harder to follow the logic. Oliver