selectrum icon indicating copy to clipboard operation
selectrum copied to clipboard

Seperate face for directories and files

Open mohkale opened this issue 3 years ago • 6 comments

Something I quite miss from ivy is the ability to quickly distinguish files from directories using colors. Ivy uses ivy-subdir face to color directoriesin ivy-find-file. I'd like selectrum to have something similair.

Screenshot_20210111_024421

mohkale avatar Jan 11 '21 02:01 mohkale

I think this could be a job for the upcoming affixation-function (Emacs 28). @minad @oantolin Dealing with this seems like a good fit for marginalia? Related: I reported a bug to improve the affixation/annotation face handling, though this doesn't directly affect how we proceed with this here.

@mohkale For now you can do the following to get the desired behaviour:

(setq selectrum-highlight-candidates-function
      (defun highlight-matches-and-dirs+ (input cands)
        (let ((cands (if (eq 'file (completion-metadata-get
                                    (completion-metadata
                                     input
                                     minibuffer-completion-table
                                     minibuffer-completion-predicate)
                                    'category))
                         (cl-loop for cand in cands
                                  for len = (length cand)
                                  if (and (> len 0)
                                          (eq (aref cand (1- len)) ?/))
                                  collect (progn (add-face-text-property
                                                  0 (length cand)
                                                  'success
                                                  'append cand)
                                                 cand)
                                  else
                                  collect cand)
                       cands)))
          (<your-default-highlight-function> input cands))))

clemera avatar Jan 11 '21 10:01 clemera

All right, I've managed to get it working thanks to @clemera, I had some issues due to prescient overriding my selectrum-highlight-candidates-function so I just adapted it for prescient only:

(use-package selectrum-prescient
  :ensure t
  :defer  t
  :hook (selectrum-mode . selectrum-prescient-mode)
  :config
  (defun selectrum-highlight-candidates-function+ (input cands)
    (let ((cands (if (eq 'file (completion-metadata-get
                                (completion-metadata
                                 input
                                 minibuffer-completion-table
                                 minibuffer-completion-predicate)
                                'category))
                     (cl-loop for cand in cands
                              for len = (length cand)
                              if (and (> len 0)
                                      (eq (aref cand (1- len)) ?/))
                              collect (progn (add-face-text-property
                                              0 (length cand)
                                              'dired-directory
                                              'append cand)
                                             cand)
                              else
                              collect cand)
                   cands)))
      (selectrum-prescient--highlight input cands)))

  (add-hook 'selectrum-prescient-mode-hook
            (defun selectrum-prescient-custom-highlight+ ()
              (setq selectrum-highlight-candidates-function
                    #'selectrum-highlight-candidates-function+))))

This works pretty well but it doesn't mix well with my selectrum-current-candidate face. I've had similair issues with marginalia (See here).

Screenshot_20210112_194543

In ivy it just highlights the entire line for the current candidate with the same face, ensuring readability. What're your thoughts on adding a new option selectrum-override-current-candidate-face or a face selectrum-override-face which takes precedence over other faces (eg. marginalia-file-modes) when on the current candidate?

mohkale avatar Jan 12 '21 19:01 mohkale

this is a similair issue in consult-line as well.

Compare swiper

Screenshot_20210112_195209

with consult-line.

Screenshot_20210112_195211

mohkale avatar Jan 12 '21 19:01 mohkale

@mohkale For comparison can you please cross check with a theme like modus-operandi which has full support for selectrum, prescient, orderless, consult etc?

minad avatar Jan 12 '21 20:01 minad

@minad

I'm not sure what you'd like me to check, but here's a similair comparison.

Screenshot_20210112_200918

NOTE: Title keeps original background.

Screenshot_20210112_200954

Screenshot_20210112_201115

The original foreground of each of the original characters is unchanged, like before. This theme seems to have a nicer contrast between different colors but the issue still remains. When the contrast isn't that good (eg. last image) it hurts readability.

Here's the same affect with ivy.

Screenshot_20210112_201715

Screenshot_20210112_201746

Screenshot_20210112_201940

With ivy the entire line of each input recieves the same face which makes it predictable and consistent.

S.N. Here's my theme file (it's custom so I guess its fair to say it doesn't yet have selectrum support)

mohkale avatar Jan 12 '21 20:01 mohkale

This makes selectrum-current-candidate face highlighting more like ivy:

(defvar selectrum-override-current-candidate-face+ t
  "When true selectrum overrides any existing faces on the current candidate
      with `selectrum-current-candidate'.")

(advice-add 'selectrum--add-face :around
            (defun selectrum-override-current-candidate-face+ (func str face)
              (if (and selectrum-override-current-candidate-face+
                       (eq face 'selectrum-current-candidate))
                  (progn
                    (setq str (copy-sequence str))
                    ;; Adapted from `selectrum--add-face' but prepend instead of append.
                    (if (version< emacs-version "27")
                        (font-lock-prepend-text-property 0 (length str) 'face face str)
                      (add-face-text-property 0 (length str) face nil str))
                    str)
                (funcall func str face))))

But there're some edge cases. Eg.

describe-face with selectrum vs. counsel-describe-face .

Screenshot_20210112_203052

Screenshot_20210112_203127

It might be more apropriate to let the highlight function know which candidate is current and then leave it upto them to override the face as appropriate. That's why I initially opened the issue on marginalia.

S.N. This approach also overrides prescient highlighting :sob:.

mohkale avatar Jan 12 '21 20:01 mohkale