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

Highlighting of selected candidate when using the display property #411

Closed
oantolin opened this issue Jan 28, 2021 · 11 comments
Closed

Highlighting of selected candidate when using the display property #411

oantolin opened this issue Jan 28, 2021 · 11 comments

Comments

@oantolin
Copy link

This is noticeable in consult-line, where the line numbers are not highlighted, or in embark-keymap-help, where the keybindings are not highlighted. Ivy does highlight the whole thing including the parts in a display text property, so it is at least possible.

@clemera
Copy link
Collaborator

clemera commented Jan 28, 2021

Thanks Omar, please see #235, I'm closing this, see there for more discussion.

@clemera clemera closed this as completed Jan 28, 2021
@clemera
Copy link
Collaborator

clemera commented Jan 28, 2021

This comment in particular there was a lot of other discussion going on, as well.

@clemera clemera changed the title Display text properties are not highlighted with the rest of the candidate Highlighting of selected candidate when using the display property Jan 28, 2021
@clemera clemera reopened this Jan 28, 2021
@clemera
Copy link
Collaborator

clemera commented Jan 28, 2021

It seemed better to me to rename the other issue to keep track of the bleeding prompt issue mentioned there and making this one the canonical one for the display property problem now.

@oantolin
Copy link
Author

Well, ivy--add-face is different from selectrum--add-face, but from just reading them I never would have guess one highlighted the display text and one didn't:

(defun ivy--add-face (str face)
  "Propertize STR with FACE."
  (let ((len (length str)))
    (condition-case nil
        (progn
          (colir-blend-face-background 0 len face str)
          (let ((foreground (face-foreground face)))
            (when foreground
              (add-face-text-property
               0 len (list :foreground foreground) nil str))))
      (error
       (ignore-errors
         (font-lock-append-text-property 0 len 'face face str)))))
  str)

@oantolin
Copy link
Author

It seems even replacing selectrum--add-face with that function doesn't fix the issue.

@clemera
Copy link
Collaborator

clemera commented Jan 29, 2021

This is weird, I thought when I tested with font-lock-append-text-property it worked but now I can't reproduce it any more, I used the following for testing:

(completing-read "Candidates: "
                 `(,(propertize "GNU" 'display "some-prefix: GNU")
                   ,(propertize "Emacs" 'display "other-prefix: Emacs")))

@clemera
Copy link
Collaborator

clemera commented Jan 29, 2021

I think the reason it works with ivy is that it doesn't use an overlay to display the candidates there seems to be a display bug with that combination in the minibuffer. I have implemented a workaround in #413, note with that the line prefix in consult-line still isn't included in the selection highlight, this could be done by always using font-lock-prepend-text-property but I wonder if it isn't actually nicer to not have it included, this is also how hl-line mode works with enabled line numbers.

@minad
Copy link
Contributor

minad commented Jan 29, 2021

I actually like it without the highlighting. I am only using 'display in consult where I am okay with no highlighting. In other cases I use 'invisible.

@oantolin
Copy link
Author

I also think the line numbers in consult-line look fine without the highlighting. It's my horrible, hacky embark-keymap-help that needs some cosmetic help.

@clemera
Copy link
Collaborator

clemera commented Jan 29, 2021

I will leave it as is then, the bleeding is fixed and the selection with embark-keymap-help works. I will still also report the display problem to the mailing list.

@clemera
Copy link
Collaborator

clemera commented Jan 29, 2021

Emacs bug report can be found here

@clemera clemera closed this as completed Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants