-
-
Notifications
You must be signed in to change notification settings - Fork 338
Preserve text properties of prompt #3077
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
Conversation
basil-conto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the pull request! And sorry for the long delay.
| (set-text-properties 0 (length n-str) | ||
| `(face minibuffer-prompt ,@std-props) | ||
| n-str) | ||
| (let ((old-props (when (fboundp 'object-intervals) | ||
| (object-intervals n-str)))) | ||
| (set-text-properties 0 (length n-str) | ||
| `(face minibuffer-prompt ,@std-props) | ||
| n-str) | ||
| (dolist (propdata old-props) | ||
| (cl-destructuring-bind (beg end props) propdata | ||
| (add-text-properties beg end props n-str)))) | ||
| (setq n-str (funcall ivy-set-prompt-text-properties-function | ||
| n-str std-props)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right after the set-text-properties call, there is a call to the variable ivy-set-prompt-text-properties-function, which is allowed to modify the prompt in-place.
Can you please describe in more detail what use-case you are considering, that can't already be addressed via ivy-set-prompt-text-properties-function?
In general, I would prefer to avoid relying on object-intervals, or needing to copy each interval over manually. At least, not by default, and not if we can think of another solution.
For example, set-text-properties could be replaced with add-text-properties. Or, std-props could be turned into a variable that can be bound around relevant calls to Ivy. But I would like to better understand the motivation before making such changes.
ivy--insert-prompt seems to emulate some of read-from-minibuffer's text property logic, but hasn't been doing so correctly w.r.t minibuffer-prompt-properties and in particular the face property. This change allows users to affect the entire prompt via m-p-p, or just the prompt text itself by passing, e.g., a propertized string to ivy-read. It should support multiple faces too. * ivy.el (ivy--insert-prompt): Break text property logic out... (ivy--propertize-prompt): ...to this new function. Use add-text-properties and add-face-text-property in place of set-text-properties (#3077). Heed minibuffer-prompt-properties.
|
In commit 4b1df7c I generalised the current code to use This doesn't preserve all text properties, but it does preserve the same text properties as Does that address your use-case, or is a new mechanism still needed? Thanks. |
This comment was marked as resolved.
This comment was marked as resolved.
This continues from the previous commit of 2025-11-22 "Don't completely replace prompt text properties". * ivy.el (ivy-set-prompt-text-properties-default): Retain existing faces by using add-face-text-property (#3077).
|
I tested new version and it works. So this PR could be closed. (read-buffer (format-prompt "select buffer (\\`C-t' to toggle filtering)" nil))To suggest a non standard key binding to user. |
Great, thank you for confirming, and once again for your patience. |
Standard Emacs' functions like
read-from-minibuffersupports propertized prompts.But
ivyoverrides text properties of prompt here:swiper/ivy.el
Line 3266 in 2257a9d
This pull request is the fix.