From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dario Gjorgjevski Newsgroups: gmane.emacs.bugs Subject: bug#42149: Substring and flex completion ignore implicit trailing =?UTF-8?Q?=E2=80=98any=E2=80=99?= Date: Mon, 28 Dec 2020 11:17:40 +0100 Message-ID: References: <87k0znsdjb.fsf@gmail.com> <87sgbsv7gg.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40390"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 42149@debbugs.gnu.org, =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Dec 28 11:18:14 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ktpbS-000APY-71 for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 28 Dec 2020 11:18:14 +0100 Original-Received: from localhost ([::1]:43464 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ktpbR-0000Uc-1d for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 28 Dec 2020 05:18:13 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:48316) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ktpbG-0000UQ-10 for bug-gnu-emacs@gnu.org; Mon, 28 Dec 2020 05:18:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:50941) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ktpbF-00031y-QE for bug-gnu-emacs@gnu.org; Mon, 28 Dec 2020 05:18:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ktpbF-0006EV-K3 for bug-gnu-emacs@gnu.org; Mon, 28 Dec 2020 05:18:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Dario Gjorgjevski Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 28 Dec 2020 10:18:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 42149 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 42149-submit@debbugs.gnu.org id=B42149.160915067523942 (code B ref 42149); Mon, 28 Dec 2020 10:18:01 +0000 Original-Received: (at 42149) by debbugs.gnu.org; 28 Dec 2020 10:17:55 +0000 Original-Received: from localhost ([127.0.0.1]:34254 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ktpb6-0006E4-BN for submit@debbugs.gnu.org; Mon, 28 Dec 2020 05:17:54 -0500 Original-Received: from mail-ej1-f42.google.com ([209.85.218.42]:39677) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ktpb1-0006Do-LQ for 42149@debbugs.gnu.org; Mon, 28 Dec 2020 05:17:51 -0500 Original-Received: by mail-ej1-f42.google.com with SMTP id n26so13563089eju.6 for <42149@debbugs.gnu.org>; Mon, 28 Dec 2020 02:17:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=uVUiuA/iCAIqhPwq/v8P9V9lRr1t8ovR0xSCPUDBIXc=; b=q0p8NG3PVSN9pmyOIofxXqHBFS60zj6edCdIPbDSls459JkBrHwiGjWVZSSQhHwITF pSdxQlDhQw/47YTo/UyxwdTq6WSJZWD9m5VEFE6nWepXMob/sPkL8XWSsIL3/VV3Jemi 2uDb+apAIIaY6e1pCLyYs4JjCNm/zC011cWexo/wd/HAEC+SM35ROOXMHrufRY1Tlj7s 4bLQRT1TcWESN4HJYpLKWKDHSRYiYcsaqf9PzAPaShI11kQehz1r9IwRuWcT/jzIksLy B7g6dtffwJmYOdQDW3GSQZRVHkHo9cozGZ4cDfu41bl/Qi3dHOxIIWG7HjlTqv0zapf5 Bu3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=uVUiuA/iCAIqhPwq/v8P9V9lRr1t8ovR0xSCPUDBIXc=; b=T11U/HPmmk1T++IIS828dxxSiGQekKG2AxDzkatXMEk+OdeZfEIHaD9U1lx6RkQ7ki YbPDOMgzgFdE0lgKRFfIuEBzbvZj/F3cJWWg6Uwg8MZtFBF1hO5/pWL/qnLLKuHLIJhN BZnDqAb5LPTgtkW0CPZG8+jwCplQa4e/1HyIl3vUNpENgQd9/IM4FVC0wIi4ATnUrR+F ZzVZZOW2kXg/kAay5ceZpJebFxluIevNGw4JQUKdwr2ePMjusvG9eitiCmvDKfckx/iG +9cMIxxv161xxSbQUAFWXHoTw5LZnu5iOUfpLoG75JCAFpKJhMoP8i+znRP9w2nCiYDK 5EHg== X-Gm-Message-State: AOAM532vibekQWs+xv7XBlJw05F4EYuXaPWaP30LYdhyoP5QWlWmdssn cooAuuZPC7Q/8NF01MifnVndJn6eL8I= X-Google-Smtp-Source: ABdhPJxT2keAZRDK9+ituqoqPtlw77JC2OiLTcUaG055UEX2DE4On8vpDOTgkF2ASTnwpgIwKv1dfQ== X-Received: by 2002:a17:906:4809:: with SMTP id w9mr40841875ejq.139.1609150661640; Mon, 28 Dec 2020 02:17:41 -0800 (PST) Original-Received: from ZALANDO-31298 ([79.140.114.243]) by smtp.gmail.com with ESMTPSA id l1sm16806231eje.12.2020.12.28.02.17.41 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 28 Dec 2020 02:17:41 -0800 (PST) In-Reply-To: (Stefan Monnier's message of "Sun, 27 Dec 2020 16:45:07 -0500") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:196856 Archived-At: Hi Stefan, > Thanks very much for your patch. > It looks good to me, but I think it's important we find a fix with which > Jo=C3=A3o agrees. Thanks likewise! >> Furthermore, =E2=80=98completions-first-difference=E2=80=99 and >> =E2=80=98completions-common-part=E2=80=99 would sometimes overlap depend= ing on the >> position of point within the query string. > > Could you point us at the corresponding test? That would be the test ;; Point is at beginning, but `completions-first-difference' is ;; moved after it (should (equal (get-text-property 1 'face (car (completion-pcm-all-completions "f" '("few" "many") nil 0))) 'completions-first-difference)) You can replace pcm with flex or substring, it doesn=E2=80=99t matter. >> The former is fixed by correcting the part of >> =E2=80=98completion-pcm--hilit-commonality=E2=80=99 responsible for loop= ing over the >> holes in the query string. > > Is that done by treating the "leftover" from the string as if there was > an additional match? That would correspond to the "implicit any" > that terminates every pattern. I believe the simplest way to summarize the issue with the current implementation is that it assumes the regex match is of the form ... (Where the s might be of length 0.) However, the trailing is not there and therefore the score is not updated correctly. Furthermore, it does nothing to actually ensure these assumptions in the presence of wildcards in the query string. >> The latter is fixed by explicitly moving >> the position of =E2=80=98completions-first-difference=E2=80=99 in case a= n overlap with >> =E2=80=98completions-common-part=E2=80=99 is detected. > > Did you (by any chance) figure out how/why the two end up overlapping? > The fix you're using looks pretty "hackish" and introduces a non-trivial > data flow for `pos`. Before using such an ad-hoc solution it'd be best > to understand where the problem comes from (it might still be the > better answer in the end, but it's hard to judge). `completions-first-difference' is put at the first position after point in the query string. However, the part of the query string *after* point might actually match there. I don=E2=80=99t see an easier solution. >> (completion-pcm--optimize-pattern): Turn multiple consecutive >> occurrences of =E2=80=98any=E2=80=99 into just a single one. > > This is good (consecutive `any` can introduce serious performance bugs > because of our backtracing regexp matcher). > Other than improving performance, have you found other effects? Yes, the presence of multiple consecutive wildcards invalidates the aforementioned assumption of completion-pcm--hilit-commonality that the match is of the form ... >> +(defun completion-pcm--count-leading-holes (pattern) >> + "Count the number of leading holes in PATTERN." >> + (length (seq-take-while #'symbolp pattern))) > > `seq-take-while` is not defined at this stage. > [...] > - Mark `seq-take-while` with a `;;;###autoload` cookie so it'll be > loaded on demand. > [...] Good catch, this should indeed be done. Best regards, Dario --=20 $ keyserver=3Dhkps://hkps.pool.sks-keyservers.net $ keyid=3D744A4F0B4F1C9371 $ gpg --keyserver $keyserver --search-keys $keyid