Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(fix)Format completions to window-width, not frame-width #2040

Merged
merged 3 commits into from
Jan 16, 2022

Conversation

stuhlmueller
Copy link
Contributor

Format minibuffer completion candidates using window-width instead of frame-width. This way completions render as a single line even if the frame is split.

Before: Empty line between completions
frame-width

After: No empty line between completions
window-width

Format minibuffer completion candidates using window-width instead of frame-width to render as single line even if frame is split
@jethrokuan
Copy link
Member

@stuhlmueller see #1578 for some discussion about frame-width vs window-width.

The TLDR is that window-width seems to only be the optimal value for Helm because of the way completions are presented in Helm. frame-width works better for every other completion framework that uses the mini-buffer, which are usually frame-width wide.

@stuhlmueller
Copy link
Contributor Author

Ah, thanks! Any reason not to merge the workaround mentioned there?

@jethrokuan
Copy link
Member

Ah, thanks! Any reason not to merge the workaround mentioned there?

Think it slipped me. Modify this PR to the workaround and I'll merge it?

@stuhlmueller
Copy link
Contributor Author

Done!

@jethrokuan jethrokuan merged commit 001de3a into org-roam:master Jan 16, 2022
@PRESFIL
Copy link
Contributor

PRESFIL commented Jan 27, 2022

I'm a little late, but this PR messes up Vertico's work, especially because Vertico always
runs in a minibuffer. This happens when the current window is halved and is smaller than
the org-roam-node-display-template requires. Perhaps bufferp current-buffer is not
working as intended.

Might be worth a try replace (bufferp current-buffer) with (minibufferp)?

-                         (1- (if (bufferp (current-buffer))
+                         (1- (if (minibufferp) 

My org-roam-node-display-template:

(org-roam-node-display-template (concat "${title:*} " (propertize "${tags:100}" 'face 'org-tag)))

You can try variant paste this into init.el:

+  (defun my/org-roam-node-read--to-candidate (node template)
+    "Return a minibuffer completion candidate given NODE.
+  TEMPLATE is the processed template used to format the entry."
+    (let ((candidate-main (org-roam-node--format-entry
+                           template
+                           node
+                           (1- (if (minibufferp)
+                                   (window-width) (frame-width))))))
+      (cons (propertize candidate-main 'node node) node)))
+
+  (advice-add 'org-roam-node-read--to-candidate :override #'my/org-roam-node-read--to-candidate))

I would like this to be tried by users of different completion engines (Helm, vertico, whatever), because
this problem is repeated in a circle, every time 😞 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants