unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks
@ 2024-11-18 17:33 Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-11-18 17:36 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 6+ messages in thread
From: Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-18 17:33 UTC (permalink / raw)
  To: 74420; +Cc: monnier


Assuming one has an Emacs repo at ~/src/emacs/trunk,

1. emacs -Q
2. C-x C-f C-a C-k ~/src/emacs/trunk/*/minibuf TAB
3. The minibuffer contents change to
   ~/src/emacs/trunk//minibuf
4. Since there's a //, subsequent file name completion tries to complete
   on the root directory, which is not what should happen.

A more concise reproduction:

(completion-pcm-try-completion
 "*/minibuf" '("lisp/minibufA" "src/minibufB") nil (length "*/minibuf"))

evalutes to

("/minibuf" . 8)

We should be preserving the *.  A patch to fix this will follow.


In GNU Emacs 31.0.50 (build 18, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2024-11-18 built on
 igm-qws-u22796a
Repository revision: 6610135a50ac2fd0683a5e467fe33faa81df21b8
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.10 (Green Obsidian)

Configured using:
 'configure -C --with-gif=no --with-x-toolkit=lucid
 --with-native-compilation=yes --prefix=/home/sbaugh/prefix-jane-emacs'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LIBSELINUX
LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM
XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr compile comint ansi-osc ansi-color ring comp-run
bytecomp byte-compile comp-common rx emacsbug message mailcap yank-media
puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
xinput2 x multi-tty move-toolbar make-network-process native-compile
emacs)

Memory information:
((conses 16 107684 9391) (symbols 48 11073 0) (strings 32 32202 1340)
 (string-bytes 1 982590) (vectors 16 14035)
 (vector-slots 8 173934 7491) (floats 8 29 1) (intervals 56 284 0)
 (buffers 984 11))





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks
  2024-11-18 17:33 bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-18 17:36 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-11-18 20:39   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 6+ messages in thread
From: Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-18 17:36 UTC (permalink / raw)
  To: 74420; +Cc: monnier

[-- Attachment #1: Type: text/plain, Size: 20 bytes --]


Patch to fix this:

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Preserve-an-explicit-in-pcm-try-completion.patch --]
[-- Type: text/x-patch, Size: 5345 bytes --]

From d7377eb6abfc57552f43687aec358934b33707e6 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 18 Nov 2024 12:26:55 -0500
Subject: [PATCH] Preserve an explicit * in pcm-try-completion

An explicitly typed * has different semantics from automatically
inserted PCM wildcards, so it should be preserved on
try-completion.  We already do this in some cases, but now we do
it more.  Concretely, we do it by optimizing the PCM pattern
more aggressively to avoid having multiple wildcards in a row:
after those are removed, the existing code in
completion-pcm--merge-completions is able to preserve * in more
cases.  The additional optimization should also improve
performance.

This is especially significant for filename completion: removing
an explicit * can take us from

~/src/emacs/trunk/*/minibuf

to

~/src/emacs/trunk//minibuf

The explicit double slash is interpreted by the file name
completion table to mean "start completing from the root
directory", so deleting the * here substantially changes
semantics.

* lisp/minibuffer.el (completion-pcm--optimize-pattern): Add
more optimizations. (bug#74420)
(completion-pcm--find-all-completions): Optimize the pattern
after concatenating two subpatterns.
* test/lisp/minibuffer-tests.el (completion-pcm--optimize-pattern)
(completion-pcm-test-7): Add tests.
---
 lisp/minibuffer.el            | 20 ++++++++++++++++----
 test/lisp/minibuffer-tests.el | 30 +++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5f3f5d3ead1..e48d85b777d 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4073,8 +4073,18 @@ completion-pcm--optimize-pattern
   (let ((n '()))
     (while p
       (pcase p
-        (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
-         (setq p (cdr p)))
+        ;; Remove duplicate `any' and `prefix'
+        (`(any any . ,rest)
+         (setq p (cons 'any rest)))
+        (`(prefix prefix . ,rest)
+         (setq p (cons 'prefix rest)))
+        ;; `any' matches anything `any-delim' does, and grows the same way.
+        (`(any-delim any . ,rest)
+         (setq p (cons 'any rest)))
+        ;; Remove other wildcards found around `star' or `point'.
+        ((or `(,(and keep (or 'star 'point)) ,(or 'any 'any-delim 'prefix) . ,rest)
+             `(,(or 'any 'any-delim 'prefix) ,(and keep (or 'star 'point)) . ,rest))
+         (setq p (cons keep rest)))
         ;; This is not just a performance improvement: it turns a
         ;; terminating `point' into an implicit `any', which affects
         ;; the final position of point (because `point' gets turned
@@ -4445,8 +4455,10 @@ completion-pcm--find-all-completions
               ;;   (dolist (submatch suball)
               ;;     (push (concat submatch between newsubstring) all)))
               ))
-          (setq pattern (append subpat (list 'any (string sep))
-                                (if between (list between)) pattern))
+          (setq pattern
+                (completion-pcm--optimize-pattern
+                 (append subpat (list 'any (string sep))
+                         (if between (list between)) pattern)))
           (setq prefix subprefix)))
       (if (and (null all) firsterror)
           (signal (car firsterror) (cdr firsterror))
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index 38c2b8c4552..d988a2007cb 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -133,7 +133,19 @@ completion-pcm--optimize-pattern
   (should (equal (completion-pcm--optimize-pattern '("buf" point "f"))
                  '("buf" point "f")))
   (should (equal (completion-pcm--optimize-pattern '(any "" any))
-                 '(any))))
+                 '(any)))
+  (should (equal (completion-pcm--optimize-pattern '(any-delim "" any))
+                 '(any)))
+  (should (equal (completion-pcm--optimize-pattern '(prefix "" prefix))
+                 '(prefix)))
+  (should (equal (completion-pcm--optimize-pattern '(prefix star any))
+                 '(star)))
+  (should (equal (completion-pcm--optimize-pattern '(any point prefix "foo"))
+                 '(point "foo")))
+  ;; The `any' and `prefix' are erased because they're next to `point',
+  ;; then `point' is erased because it's at the end.
+  (should (equal (completion-pcm--optimize-pattern '(any point prefix))
+                 '())))
 
 (defun test-completion-all-sorted-completions (base def history-var history-list)
   (with-temp-buffer
@@ -258,6 +270,22 @@ completion-pcm-test-6
            (car (completion-pcm-all-completions
                  "li-pac*" '("do-not-list-packages") nil 7)))))
 
+(ert-deftest completion-pcm-test-7 ()
+  ;; Wildcards are preserved even when right before a delimiter.
+  (should (equal
+           (completion-pcm-try-completion
+            "x*/"
+            '("x1/y1" "x2/y2")
+            nil 3)
+           '("x*/y" . 4)))
+  ;; This is important if the wildcard is at the start of a component.
+  (should (equal
+           (completion-pcm-try-completion
+            "*/minibuf"
+            '("lisp/minibuffer.el" "src/minibuf.c")
+            nil 9)
+           ("*/minibuf" . 9))))
+
 (ert-deftest completion-substring-test-1 ()
   ;; One third of a match!
   (should (equal
-- 
2.39.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks
  2024-11-18 17:36 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-18 20:39   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-11-18 22:17     ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-18 20:39 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: 74420

> An explicitly typed * has different semantics from automatically
> inserted PCM wildcards, so it should be preserved on
> try-completion.  We already do this in some cases, but now we do
> it more.  Concretely, we do it by optimizing the PCM pattern
> more aggressively to avoid having multiple wildcards in a row:
> after those are removed, the existing code in
> completion-pcm--merge-completions is able to preserve * in more
> cases.

Oh, indeed, thank you.

The patch looks good to me, so if there are no objection, I'll install
it in a few days (feel free to ping me if I forget).
Banter follows.

> * lisp/minibuffer.el (completion-pcm--optimize-pattern): Add
> more optimizations. (bug#74420)

Aha, so that's why you didn't submit the patch together with the initial
bug report!

> -        (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
> -         (setq p (cdr p)))
> +        ;; Remove duplicate `any' and `prefix'
> +        (`(any any . ,rest)
> +         (setq p (cons 'any rest)))
> +        (`(prefix prefix . ,rest)
> +         (setq p (cons 'prefix rest)))
> +        ;; `any' matches anything `any-delim' does, and grows the same way.
> +        (`(any-delim any . ,rest)
> +         (setq p (cons 'any rest)))

AFAICT you can still merge the `any-delim any` and `any any` cases.

> +        ;; Remove other wildcards found around `star' or `point'.
> +        ((or `(,(and keep (or 'star 'point)) ,(or 'any 'any-delim 'prefix) . ,rest)
> +             `(,(or 'any 'any-delim 'prefix) ,(and keep (or 'star 'point)) . ,rest))
> +         (setq p (cons keep rest)))

BTW, maybe we should go with something like

       (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
        (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
              ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
              (t (push (pop p) n))))

Where `completion-pcm--<something>-p` is some kind of partial ordering?


        Stefan






^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks
  2024-11-18 20:39   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-18 22:17     ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-11-18 23:58       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 6+ messages in thread
From: Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-18 22:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 74420

[-- Attachment #1: Type: text/plain, Size: 3430 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> An explicitly typed * has different semantics from automatically
>> inserted PCM wildcards, so it should be preserved on
>> try-completion.  We already do this in some cases, but now we do
>> it more.  Concretely, we do it by optimizing the PCM pattern
>> more aggressively to avoid having multiple wildcards in a row:
>> after those are removed, the existing code in
>> completion-pcm--merge-completions is able to preserve * in more
>> cases.
>
> Oh, indeed, thank you.
>
> The patch looks good to me, so if there are no objection, I'll install
> it in a few days (feel free to ping me if I forget).

Actually... I just realized this misses some cases, namely when we have
"star point" or "point star".  Attached at the end is a different patch
which takes a different approach (and which includes new tests for the
problematic cases).

The changes to optimize still seem reasonable to me, since they might
improve performance, so feel free to install it if you think it's good.
Although maybe we want to revise it with the below discussion.

> Banter follows.
>
>> * lisp/minibuffer.el (completion-pcm--optimize-pattern): Add
>> more optimizations. (bug#74420)
>
> Aha, so that's why you didn't submit the patch together with the initial
> bug report!
>
>> -        (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_)
>> -         (setq p (cdr p)))
>> +        ;; Remove duplicate `any' and `prefix'
>> +        (`(any any . ,rest)
>> +         (setq p (cons 'any rest)))
>> +        (`(prefix prefix . ,rest)
>> +         (setq p (cons 'prefix rest)))
>> +        ;; `any' matches anything `any-delim' does, and grows the same way.
>> +        (`(any-delim any . ,rest)
>> +         (setq p (cons 'any rest)))
>
> AFAICT you can still merge the `any-delim any` and `any any` cases.

True, I just thought this was a bit easier to read, since the cases are
justified as an optimization in slightly different ways.

>> +        ;; Remove other wildcards found around `star' or `point'.
>> +        ((or `(,(and keep (or 'star 'point)) ,(or 'any 'any-delim 'prefix) . ,rest)
>> +             `(,(or 'any 'any-delim 'prefix) ,(and keep (or 'star 'point)) . ,rest))
>> +         (setq p (cons keep rest)))
>
> BTW, maybe we should go with something like
>
>        (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
>         (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
>               ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
>               (t (push (pop p) n))))
>
> Where `completion-pcm--<something>-p` is some kind of partial ordering?

Interesting thought.  Maybe it would make sense to have something like
- completion-pcm--wildcard-grows-on-left-p
  (non-nil for star, point, any, any-delim)
- completion-pcm--wildcard-grows-on-right-p
  (non-nil for star, point, prefix)
- completion-pcm--wildcard-in-text-p
  (non-nil for star and point)

Then this case would be something like "if grows-left/right-p is the
same, and at most one of the symbols is in-text-p, delete the
non-in-text-p one".

Also, then the first two helpers could be used in
completion-pcm--merge-completions as well, which could make the wildcard
behavior easier to understand.  (I want to do a bit of refactoring in
completion-pcm--merge-completions, especially after my attached change;
I think it could all be a bit easier to understand.)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Preserve-an-explicit-in-pcm-try-completion.patch --]
[-- Type: text/x-patch, Size: 5158 bytes --]

From 8c2ff1152800ede05cb9a9fd80deac45646ce52f Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 18 Nov 2024 12:26:55 -0500
Subject: [PATCH] Preserve an explicit * in pcm-try-completion

An explicitly typed * has different semantics from automatically
inserted PCM wildcards, so it should be preserved on try-completion.  We
already do this in some cases, but now we do it more.

This is especially significant for filename completion: removing an
explicit * can take us from

~/src/emacs/trunk/*/minibuf

to

~/src/emacs/trunk//minibuf

The explicit double slash is interpreted by the file name completion
table to mean "start completing from the root directory", so deleting
the * here substantially changes semantics.

* lisp/minibuffer.el (completion-pcm--merge-completions): Don't drop
important wildcards. (bug#74420)
* test/lisp/minibuffer-tests.el (completion-pcm-test-7): Add tests.
---
 lisp/minibuffer.el            | 21 +++++++++++++++------
 test/lisp/minibuffer-tests.el | 25 +++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 5f3f5d3ead1..2d27fef44ab 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4513,12 +4513,17 @@ completion-pcm--merge-completions
       ;; Then for each of those non-constant elements, extract the
       ;; commonality between them.
       (let ((res ())
-            (fixed ""))
+            (fixed "")
+            ;; Accumulate each stretch of wildcards, and process them as a unit.
+            (wildcards ()))
         ;; Make the implicit trailing `any' explicit.
         (dolist (elem (append pattern '(any)))
           (if (stringp elem)
-              (setq fixed (concat fixed elem))
+              (progn
+                (setq fixed (concat fixed elem))
+                (setq wildcards nil))
             (let ((comps ()))
+              (push elem wildcards)
               (dolist (cc (prog1 ccs (setq ccs nil)))
                 (push (car cc) comps)
                 (push (cdr cc) ccs))
@@ -4542,14 +4547,16 @@ completion-pcm--merge-completions
                     (push prefix res)
                   ;; `prefix' only wants to include the fixed part before the
                   ;; wildcard, not the result of growing that fixed part.
-                  (when (eq elem 'prefix)
+                  (when (seq-some (lambda (elem) (eq elem 'prefix)) wildcards)
                     (setq prefix fixed))
                   (push prefix res)
-                  (push elem res)
+                  ;; Push all the wildcards in this stretch, to preserve `point' and
+                  ;; `star' wildcards before ELEM.
+                  (setq res (nconc wildcards res))
                   ;; Extract common suffix additionally to common prefix.
                   ;; Don't do it for `any' since it could lead to a merged
                   ;; completion that doesn't itself match the candidates.
-                  (when (and (memq elem '(star point prefix))
+                  (when (and (seq-some (lambda (elem) (memq elem '(star point prefix))) wildcards)
                              ;; If prefix is one of the completions, there's no
                              ;; suffix left to find.
                              (not (assoc-string prefix comps t)))
@@ -4563,7 +4570,9 @@ completion-pcm--merge-completions
                                         comps))))))
                       (cl-assert (stringp suffix))
                       (unless (equal suffix "")
-                        (push suffix res)))))
+                        (push suffix res))))
+                  ;; We pushed these wildcards on RES, so we're done with them.
+                  (setq wildcards nil))
                 (setq fixed "")))))
         ;; We return it in reverse order.
         res)))))
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index 38c2b8c4552..c166450d87a 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -258,6 +258,31 @@ completion-pcm-test-6
            (car (completion-pcm-all-completions
                  "li-pac*" '("do-not-list-packages") nil 7)))))
 
+(ert-deftest completion-pcm-test-7 ()
+  ;; Wildcards are preserved even when right before a delimiter.
+  (should (equal
+           (completion-pcm-try-completion
+            "x*/"
+            '("x1/y1" "x2/y2")
+            nil 3)
+           '("x*/y" . 4)))
+  ;; Or around point.
+  (should (equal
+           (completion-pcm--merge-try
+            '(point star "foo") '("xxfoo" "xyfoo") "" "")
+           '("x*foo" . 1)))
+  (should (equal
+           (completion-pcm--merge-try
+            '(star point "foo") '("xxfoo" "xyfoo") "" "")
+           '("x*foo" . 2)))
+  ;; This is important if the wildcard is at the start of a component.
+  (should (equal
+           (completion-pcm-try-completion
+            "*/minibuf"
+            '("lisp/minibuffer.el" "src/minibuf.c")
+            nil 9)
+           '("*/minibuf" . 9))))
+
 (ert-deftest completion-substring-test-1 ()
   ;; One third of a match!
   (should (equal
-- 
2.39.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks
  2024-11-18 22:17     ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-18 23:58       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-11-19 13:18         ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-18 23:58 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: 74420

> Actually... I just realized this misses some cases, namely when we have
> "star point" or "point star".

FWIW, my local patches have included for years optimizations like the
ones you suggested *and* they replaced `star point` and `point star`
with just `star`.

Do you think it's important to preserve `point` in those cases?

>> BTW, maybe we should go with something like
>>
>>        (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
>>         (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
>>               ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
>>               (t (push (pop p) n))))
>>
>> Where `completion-pcm--<something>-p` is some kind of partial ordering?
>
> Interesting thought.  Maybe it would make sense to have something like
> - completion-pcm--wildcard-grows-on-left-p
>   (non-nil for star, point, any, any-delim)
> - completion-pcm--wildcard-grows-on-right-p
>   (non-nil for star, point, prefix)
> - completion-pcm--wildcard-in-text-p
>   (non-nil for star and point)
> Then this case would be something like "if grows-left/right-p is the
> same, and at most one of the symbols is in-text-p, delete the
> non-in-text-p one".

But I guess your `completion-pcm--merge-completions` is still needed as
long as we can't optimize all sequences of symbols to a single symbol.
🙁


        Stefan






^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks
  2024-11-18 23:58       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-19 13:18         ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 6+ messages in thread
From: Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-19 13:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 74420

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Actually... I just realized this misses some cases, namely when we have
>> "star point" or "point star".
>
> FWIW, my local patches have included for years optimizations like the
> ones you suggested *and* they replaced `star point` and `point star`
> with just `star`.
>
> Do you think it's important to preserve `point` in those cases?

Hm, I'm not sure.  I've been playing around with alternative ways to
move point in pcm-try-completion but haven't yet got something I'm
satisfied with.  So let me defer this question until I've hacked on that
some more :)

>>> BTW, maybe we should go with something like
>>>
>>>        (`(,(and (pred symbolp) s1) ,(and (pred symbolp) s2) . ,rest)
>>>         (cond ((completion-pcm--<something>-p s1 s2) (setq p (cons s1 rest)))
>>>               ((completion-pcm--<something>-p s2 s1) (setq p (cons s2 rest)))
>>>               (t (push (pop p) n))))
>>>
>>> Where `completion-pcm--<something>-p` is some kind of partial ordering?
>>
>> Interesting thought.  Maybe it would make sense to have something like
>> - completion-pcm--wildcard-grows-on-left-p
>>   (non-nil for star, point, any, any-delim)
>> - completion-pcm--wildcard-grows-on-right-p
>>   (non-nil for star, point, prefix)
>> - completion-pcm--wildcard-in-text-p
>>   (non-nil for star and point)
>> Then this case would be something like "if grows-left/right-p is the
>> same, and at most one of the symbols is in-text-p, delete the
>> non-in-text-p one".
>
> But I guess your `completion-pcm--merge-completions` is still needed as
> long as we can't optimize all sequences of symbols to a single symbol.
> 🙁

Yep.  And here's one case that I think makes optimizing all sequences
down to a single symbol impossible: (star star star) is reified as ***,
not shrunk down to a single *.  I think that behavior is probably
reasonable (or at least I'm not in any rush to get rid of it), but to
preserve it we have to preserve sequences of multiple symbols.





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-11-19 13:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-18 17:33 bug#74420: 31.0.50; PCM completion for ~/src/emacs/trunk/*/minibuf breaks Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-18 17:36 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-18 20:39   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-18 22:17     ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-18 23:58       ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-19 13:18         ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).