projectile icon indicating copy to clipboard operation
projectile copied to clipboard

[Feature] Set completion-metadata with completing-read?

Open mohkale opened this issue 4 years ago • 6 comments

I'd like projectile projectile-completing-read to add a :category parameter that callers can use to assign the category metadata for completing-read. This can intern be used by packages such as marginalia or embark to annotate and configure the completion.

Expected behavior

For example here's projectile-find-file when it passes :category 'project-file to projectile-completing-read. Annotations curtesy of marginalia.el.

Screenshot_20210517_171948

Actual behavior

It looks just like a regular completing-read session with no special highlighting.

Steps to reproduce the problem

Run projectile-find-file.

Environment & Version information

Projectile version information

Include here the version string displayed by M-x projectile-version. Here's an example:

Projectile version: Projectile 2.4.0snapshot

Emacs version

GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.29, cairo version 1.17.4) of 2021-04-30

Operating system

Linux mk-desktop 5.12.1-arch1-1 #1 SMP PREEMPT Sun, 02 May 2021 12:43:58 +0000 x86_64 GNU/Linux

Demo Patch

Used to produce the image shown above.

--- /home/mohkale/.config/dotfiles/prog/editors/emacs/lisp/straight/repos/projectile/projectile.el
+++ #<buffer projectile.el>
@@ -1870,7 +1870,7 @@
 project-root for every file."
   (expand-file-name name (projectile-project-root)))
 
-(cl-defun projectile-completing-read (prompt choices &key initial-input action)
+(cl-defun projectile-completing-read (prompt choices &key initial-input action category)
   "Present a project tailored PROMPT with CHOICES."
   (let ((prompt (projectile-prepend-project-name prompt))
         res)
@@ -1882,7 +1882,11 @@
                       ((bound-and-true-p ivy-mode)  'ivy)
                       (t 'default))
                    projectile-completion-system)
-            ('default (completing-read prompt choices nil nil initial-input))
+            ('default (completing-read prompt (lambda (string predicate action)
+                                                (if (eq action 'metadata)
+                                                    `(metadata (category . ,category))
+                                                  (complete-with-action action choices string predicate)))
+                                       nil nil initial-input))
             ('ido (ido-completing-read prompt choices nil nil initial-input))
             ('helm
              (if (and (fboundp 'helm)
@@ -2248,7 +2252,8 @@
   (projectile-maybe-invalidate-cache invalidate-cache)
   (let* ((project-root (projectile-acquire-root))
          (file (projectile-completing-read "Find file: "
-                                           (projectile-project-files project-root)))
+                                           (projectile-project-files project-root)
+                                           :category 'project-file))
          (ff (or ff-variant #'find-file)))
     (when file
       (funcall ff (expand-file-name file project-root))
@@ -2379,7 +2384,8 @@
      "Find dir: "
      (if projectile-find-dir-includes-top-level
          (append '("./") project-dirs)
-       project-dirs))))
+       project-dirs)
+     :category 'project-file)))
 
 ;;;###autoload
 (defun projectile-find-test-file (&optional invalidate-cache)

mohkale avatar May 17 '21 16:05 mohkale

Another - hackish - way to achieve that is to tell marginalia about the category:

(add-to-list 'marginalia-prompt-categories '("Find file:" . project-file))

This associates the prompt Find file: with the category project-file.

Though it was definitely more beautiful if projectile sets it by ifself.

floli avatar Oct 05 '21 17:10 floli

@floli

I think this would be a better approach just for marginalia itself.

(setq marginalia-command-categories
        (append '((projectile-find-file . project-file)
                  (projectile-find-dir . project-file)
                  (projectile-switch-project . file))
                marginalia-command-categories))

mohkale avatar Oct 05 '21 17:10 mohkale

@mohkale Note that the project-file completion category is defined by project.el (not projectile). Marginalia will internally use project.el to resolve the project root directory instead of projectile, which is not correct. In order to solve this correctly either unify the project.el/projectile root directory finding or use a projectile-specific category projectile-file.

minad avatar Oct 15 '21 22:10 minad

@minad is there any value in distinguishing them? I was under the impression project-file was for specifying this collection is a list of files from a project.

mohkale avatar Oct 15 '21 22:10 mohkale

There is a value distinguishing them if the projectile project root and the project.el project roots do not coincide due to the different resulting file paths. If the roots always coincide there is no value in distinguishing them. Note that Doom Emacs also does this, it cannot be that bad. ;)

https://github.com/hlissner/doom-emacs/blob/06dc65f906a199ebeeb3cf00b5a5b806e6c6ebcb/modules/completion/vertico/config.el#L202-L205

But ultimately it would probably be best if projectile bases its root detection facilities on project.el and only provides additional enhanced functionality on top (additional commands etc). But my judgement regarding this is limited, this should of course be left to the projectile/project.el maintainers to decide. While I used projectile for a long time, I switched to project.el with Emacs 27.

minad avatar Oct 15 '21 22:10 minad

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

stale[bot] avatar Apr 16 '22 10:04 stale[bot]