protesilaos / mct

Enhancements for the default minibuffer completion UI of Emacs.
https://protesilaos.com/emacs/mct
GNU General Public License v3.0
32 stars 3 forks source link

MCT + TAB hiding completions despite mct-persist-dynamic-completion being t #7

Open zenspider opened 3 months ago

zenspider commented 3 months ago

Given the following custom mct settings:

 '(mct-completion-passlist
   '(Info-goto-node Info-index Info-menu vc-retrieve-tag find-file-in-project ffip file))
 '(mct-hide-completion-mode-line t)
 '(mct-mode t)
 '(mct-remove-shadowed-file-names t)

When I use find-file-in-project with the following files in my project:

9975 $ fd . test
test/minitest/
test/minitest/metametameta.rb
test/minitest/test_minitest_assertions.rb
test/minitest/test_minitest_benchmark.rb
test/minitest/test_minitest_mock.rb
test/minitest/test_minitest_reporter.rb
test/minitest/test_minitest_spec.rb
test/minitest/test_minitest_stub.rb
test/minitest/test_minitest_test.rb
test/minitest/test_minitest_test_task.rb

mct immediately pops up showing all the files in the project. When I type te, the selection immediately narrows to the "test" files listed above, and then if I hit TAB, it completes the minibuffer to test/minitest/ but then hides the completions. It is kinda hard to debug because I'm in the midst of a minibuffer completion, but it looks like the problem is the logic in mct--persist-dynamic-completion, specifically (memq (mct--completion-category) mct--dynamic-completion-categories)... (mct--completion-category) is nil.

This doesn't happen with find-file (but it also doesn't narrow as nicely) and it looks like find-file might be setting some metadata that FFIP isn't setting?

Can you point me in the right direction so I can get this either 1) hacked up in mct to Just Work when ffip (I assumed adding ffip to mct-completion-passlist might count here) or some doco/code example for the metadata so I can get a PR over to redguardtoo/find-file-in-project ?

zenspider commented 3 months ago

It looks like a workaround might be to add nil to mct--dynamic-completion-categories ... but that's a static variable, not populated from mct-completion-passlist (??), nor customizable, so it doesn't feel like the right solution. Seems like maybe the memq above should be consulting mct-completion-passlist instead?

protesilaos commented 2 months ago

Hello @zenspider!

This issue slipped through the cracks. I cannot find the command find-file-in-project. Perhaps you mean project-find-file?

At any rate, I can reproduce the problem. This change fixes it for me with the project-find-file.

 mct.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mct.el b/mct.el
index b231dc8..0b2f456 100644
--- a/mct.el
+++ b/mct.el
@@ -276,7 +276,7 @@ (defun mct--blocklist-p ()
 ;; that.
 ;;
 ;; See bug#52389: <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52389>.
-(defvar mct--dynamic-completion-categories '(file)
+(defvar mct--dynamic-completion-categories '(file project-file)
   "Completion categories that perform dynamic completion.")

 ;;;; Sorting functions for `completions-sort' (Emacs 29)

The way to go about this is to find the completion category of the given prompt. Type M-: (M-x eval-expression) and then evaluate (mct--completion-category) while inside the prompt.

You need recursive minibuffers to do this. Here are my settings for that:

(use-package mb-depth
  :ensure nil
  :hook (after-init . minibuffer-depth-indicate-mode)
  :config
  (setq read-minibuffer-restore-windows nil) ; Emacs 28
  (setq enable-recursive-minibuffers t))
protesilaos commented 2 months ago

Just to add that we can then make the mct--dynamic-completion-categories a user option.

zenspider commented 2 months ago

FFIP = https://melpa.org/#/find-file-in-project ... I am not sure it matters as it is just using completing-read but I don't know how the category stuff gets determined.

I'm not a big fan of recursive minibuffers... is mct--dynamic-completion-categories meant to be private (--) or it is meant to be set in a dir-local-variable? Feels like something that should be pulled up to defcustom to me, but I don't know the original intent.

protesilaos commented 2 months ago

From: Ryan Davis @.***> Date: Sat, 27 Apr 2024 12:51:05 -0700

FFIP = https://melpa.org/#/find-file-in-project ... I am not sure it matters as it is just using completing-read but I don't know how the category stuff gets determined.

The completion category is metadata associated with the completion table passed to 'completing-read'. I checked and the 'find-file-in-project' does not set any such category.

Completion categories are used to set completion styles, with 'completion-category-overrides'. They are also used by packages like 'marginalia' and 'embark'.

I'm not a big fan of recursive minibuffers...

Sure. This was just to determine the category, not a permanent change.

is mct--dynamic-completion-categories meant to be private (--) or it is meant to be set in a dir-local-variable? Feels like something that should be pulled up to defcustom to me, but I don't know the original intent.

I kept it private because I did not know what the scope of this feature would end up being. You can see this in the documentation of the 'mct-persist-dynamic-completion', which mentions 'find-file' and the 'file' completion category.

The question then is whether we want to change how this dynamic completion works, or review what happens with commands that are added to the 'mct-completion-passlist'. I think the latter is probably what we want.

-- Protesilaos Stavrou https://protesilaos.com

protesilaos commented 2 months ago

This change makes passlist items persist their completions:

 mct.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mct.el b/mct.el
index b231dc8..a2e9f3e 100644
--- a/mct.el
+++ b/mct.el
@@ -1044,7 +1044,8 @@ (defun mct--persist-dynamic-completion (&rest _)
   "Persist completion, per `mct-persist-dynamic-completion'."
   (when (and (not (mct--symbol-in-list mct-completion-blocklist))
              mct-persist-dynamic-completion
-             (memq (mct--completion-category) mct--dynamic-completion-categories)
+             (or (memq (mct--completion-category) mct--dynamic-completion-categories)
+                 (mct--passlist-p))
              mct-live-completion)
     (mct-focus-minibuffer)
     (mct--show-completions)))
protesilaos commented 2 months ago

I will rewrite that function now, but the above diff proves the concept.

protesilaos commented 2 months ago

Here:

 mct.el | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/mct.el b/mct.el
index b231dc8..d586802 100644
--- a/mct.el
+++ b/mct.el
@@ -1040,23 +1040,32 @@ (defun mct--setup-completion-list ()

 ;;;;; Dynamic completion

-(defun mct--persist-dynamic-completion (&rest _)
-  "Persist completion, per `mct-persist-dynamic-completion'."
-  (when (and (not (mct--symbol-in-list mct-completion-blocklist))
-             mct-persist-dynamic-completion
-             (memq (mct--completion-category) mct--dynamic-completion-categories)
-             mct-live-completion)
+(defun mct-persist-completions-buffer (&rest _)
+  "Persist the completions buffer if there are still candidates.
+Do this under any of the following conditions:
+
+- The command is in the `mct-completion-passlist'.
+
+- The `mct-live-completion' is non-nil.
+
+- The `mct-persist-dynamic-completion' is non-nil and the current
+  command performs dynamic completion (like `find-file')."
+  (when (and (not (mct--blocklist-p))
+             (or (and mct-persist-dynamic-completion
+                      (memq (mct--completion-category) mct--dynamic-completion-categories))
+                 (mct--passlist-p)
+                 mct-live-completion))
     (mct-focus-minibuffer)
     (mct--show-completions)))

-(defun mct--setup-dynamic-completion-persist ()
-  "Set up `mct-persist-dynamic-completion'."
+(defun mct--setup-persistent-completions ()
+  "Set up `mct-persist-completions-buffer'."
   (let ((commands '(choose-completion minibuffer-complete minibuffer-force-complete)))
     (if (bound-and-true-p mct-mode)
         (dolist (fn commands)
-          (advice-add fn :after #'mct--persist-dynamic-completion))
+          (advice-add fn :after #'mct-persist-completions-buffer))
       (dolist (fn commands)
-        (advice-remove fn #'mct--persist-dynamic-completion)))))
+        (advice-remove fn #'mct-persist-completions-buffer)))))

 ;;;;; mct-mode declaration

@@ -1084,7 +1093,7 @@ (define-minor-mode mct-mode
     (advice-remove #'completing-read-multiple #'mct--crm-indicator)
     (advice-remove #'minibuffer-completion-help #'mct--minibuffer-completion-help-advice)
     (advice-remove #'minibuf-eldef-setup-minibuffer #'mct--stealthily))
-  (mct--setup-dynamic-completion-persist)
+  (mct--setup-persistent-completions)
   (mct--setup-message-advices))

 (define-obsolete-function-alias
zenspider commented 2 months ago

Thank you for the additional info! I've added an issue to ffip to see about getting them to define a category. Link above.