From 2c699b87c2e4341be30908368eda7237c34a0152 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jo=C3=A3o=20T=C3=A1vora?= Date: Thu, 19 Aug 2021 11:04:37 +0100 Subject: [PATCH] Improve fix of bug#49888 on no-pattern flex sorting This version is functionally equivalent, but doesn't duplicate any code. When nothing "flexy" is happening, it works by simply not doing any metadata adjustments, instead of attempting to synthesize a function to mimic the non-flex case. * lisp/minibuffer.el (completion--flex-adjust-metadata): Simplify. --- lisp/minibuffer.el | 69 +++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 41 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index f335a9e13b4..ffcd5d88abe 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -3943,51 +3943,38 @@ that is non-nil." (put 'flex 'completion--adjust-metadata 'completion--flex-adjust-metadata) (defun completion--flex-adjust-metadata (metadata) - (cl-flet - ((compose-flex-sort-fn - (existing-sort-fn) ; wish `cl-flet' had proper indentation... - (lambda (completions) - (cond - (;; Sort by flex score whenever outside the minibuffer or - ;; in the minibuffer with some input. JT@2019-12-23: - ;; FIXME: this is still wrong. What we need to test here - ;; is "some input that actually leads to flex filtering", - ;; not "something after the minibuffer prompt". Among - ;; other inconsistencies, the latter is always true for - ;; file searches, meaning the next clauses in this cond - ;; will be ignored. - (or (not (window-minibuffer-p)) - (> (point-max) (minibuffer-prompt-end))) + "If `flex' is actually doing filtering, adjust sorting." + (let ((flex-is-filtering-p + ;; JT@2019-12-23: FIXME: this is kinda wrong. What we need + ;; to test here is "some input that actually leads/led to + ;; flex filtering", not "something after the minibuffer + ;; prompt". E.g. The latter is always true for file + ;; searches, meaning we'll be doing extra work when we + ;; needn't. + (or (not (window-minibuffer-p)) + (> (point-max) (minibuffer-prompt-end)))) + (existing-dsf + (completion-metadata-get metadata 'display-sort-function)) + (existing-csf + (completion-metadata-get metadata 'cycle-sort-function))) + (cl-flet + ((compose-flex-sort-fn + (existing-sort-fn) ; wish `cl-flet' had proper indentation... + (lambda (completions) (sort - (if existing-sort-fn - (funcall existing-sort-fn completions) - completions) + (funcall existing-sort-fn completions) (lambda (c1 c2) (let ((s1 (get-text-property 0 'completion-score c1)) (s2 (get-text-property 0 'completion-score c2))) - (> (or s1 0) (or s2 0)))))) - (;; If no existing sort fn and nothing flexy happening, use - ;; the customary sorting strategy. - ;; - ;; JT@2021-08-15: FIXME: ideally this wouldn't repeat - ;; logic in `completion-all-sorted-completions', but that - ;; logic has other context that is either expensive to - ;; compute or not easy to access here. - (not existing-sort-fn) - (let ((lalpha (minibuffer--sort-by-length-alpha completions)) - (hist (and (minibufferp) - (and (not (eq minibuffer-history-variable t)) - (symbol-value minibuffer-history-variable))))) - (if hist (minibuffer--sort-by-position hist lalpha) lalpha))) - (t (funcall existing-sort-fn completions)))))) - `(metadata - (display-sort-function - . ,(compose-flex-sort-fn - (completion-metadata-get metadata 'display-sort-function))) - (cycle-sort-function - . ,(compose-flex-sort-fn - (completion-metadata-get metadata 'cycle-sort-function))) - ,@(cdr metadata)))) + (> (or s1 0) (or s2 0)))))))) + `(metadata + ,@(and flex-is-filtering-p + `((display-sort-function + . ,(compose-flex-sort-fn (or existing-dsf #'identity))))) + ,@(and flex-is-filtering-p + `((cycle-sort-function + . ,(compose-flex-sort-fn (or existing-csf #'identity))))) + ,@(cdr metadata))))) (defun completion-flex--make-flex-pattern (pattern) "Convert PCM-style PATTERN into PCM-style flex pattern. -- 2.39.2