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

Apply a "transformation" to all completing-read functions that use file candidates #181

Closed
terlar opened this issue Aug 19, 2020 · 14 comments

Comments

@terlar
Copy link

terlar commented Aug 19, 2020

I was just trying to see if I could get all-the-icons integration with selectrum and it seems to go well.

I defined the following function:

(defun completing-read-file-candidate (candidate)
  "Format `completing-read' file CANDIDATE with `abbreviate-file-name'
and icon prefix from `all-the-icons-icon-for-file'."
  (propertize (abbreviate-file-name candidate)
              'selectrum-candidate-display-prefix
              (concat (all-the-icons-icon-for-file candidate) "\t")))

Then it can be used with a map-car on the file-lists. But I wonder how do I integrate it with all the different functions?

For my own custom functions it is fine:

(defun recentf-open-files+ ()
  "Use `completing-read' to open a recent file."
  (interactive)
  (let ((files (cl-mapcar #'completing-read-file-candidate recentf-list)))
    (find-file (completing-read "Find recent file: " files nil t))))

But if I want to integrate it with find-file, find-file-rg and all the other fancy functions that exist that list files. What is the best way?

@clemera
Copy link
Collaborator

clemera commented Aug 19, 2020

There is currently no general way to do that but see #178 and #156 for more discussion on how this could be handled.

@terlar terlar changed the title Apply a "filter" to all completing-read functions that use file candidates Apply a "transformation" to all completing-read functions that use file candidates Aug 20, 2020
@raxod502
Copy link
Member

Indeed. I think it would be nice if there were a safe, standard way to make changes like this.

@haji-ali
Copy link
Contributor

haji-ali commented Aug 27, 2020

I was playing around with this method and one problem with it is that it is very slow because the formatting (adding the icons in this case) is done before filtering. Which means that all candidates are formatted even when only a handful are shown (due to search or space constraints).

It would be more helpful to apply the transformation after the filtering/truncation of the list but before displaying the candidates. I hope that you would consider adding this functionality.

EDIT: Additionally, the selectrum-candidate-display-prefix is not highlighted when the corresponding items is selected in the list.

@terlar
Copy link
Author

terlar commented Aug 27, 2020

Great ideas, I was thinking I felt some lag, but have mostly been using it with the recent files which I have limited to 200 candidates, so quite speedy non-the-less. But I do agree it makes more sense if possible if it only applies to the shown candidates (for performance).

Regarding your edit, for me even the selectrum-candidate-display-prefix is highlighted during the selection of the items.

@haji-ali
Copy link
Contributor

@terlar perhaps your computer is faster than mine since I also limit the recent files to 200, but the lag is definitely noticeable.

Regarding the highlighting, it's my mistake. I change the code to use the display property like all-the-icons-ivy and that made the highlighting not extend to the the icon. Your original code does not have this issue.

@terlar
Copy link
Author

terlar commented Aug 28, 2020

Did you try without the abbreviate-file-name BTW, I suspect that is the slower part. I know I had that issue in ivy when I wanted to format the git candidates in the same way as recentf not to get duplicate files (I was using a combination of git candidates + recentf). With that one if I had 50000 files or more it was very slow using the abbreviate-file-name so I had to resort to this to make it quick:

(defun counsel-maybe-git-cands ()
    (let ((root (counsel--git-root)))
      (when root
        (let ((concat-root-with
               (apply-partially 'concat (abbreviate-file-name root))))
          (cl-mapcar concat-root-with (counsel-git-cands root))))))

It actually seems like recentf might have them properly formatted already so maybe it is not even needed?

@haji-ali
Copy link
Contributor

haji-ali commented Aug 28, 2020

EDIT: Turns out my own code was the bottleneck and once I fixed it things became much faster (except for the initial formatting which does take around half a second, but that's manageable).

@clemera
Copy link
Collaborator

clemera commented Dec 12, 2020

There is now the marginalia (ping @minad) package which allows you to add annotations based on metadata. To add support for file icons we (and marginalia?) need to add support for the upcoming affixation-function (see #240). The annotations are added on display so this will also be more efficient.

@minad
Copy link
Contributor

minad commented Dec 12, 2020

The point of the affixation function is to have prefix and suffix annotations? I wonder why you say they are more efficient, only because the annotations are applied in batch? But hopefully it will still be lazy?

There is alread minad/marginalia#9. As soon as affixation functions become available, we will support them.

@clemera
Copy link
Collaborator

clemera commented Dec 12, 2020

The point of the affixation function is to have prefix and suffix annotations? I wonder why you say they are more efficient, only because the annotations are applied in batch? But hopefully it will still be lazy?

I mentioned that because when I skim read the rest of this discussion there was mentioned that adding icons can be slow. As we only apply annotations on display now this problem should go away. The downside is that it has to be redone on every display but I hope it's not that slow that this would introduce problems.

As soon as affixation functions become available, we will support them.

Do you mean when Emacs 28 comes out or when we add support here? Should be easy to add, I will have a look.

@minad
Copy link
Contributor

minad commented Dec 12, 2020

Do you mean when Emacs 28 comes out or when we add support here? Should be easy to add, I will have a look.

Well, icomplete will start to support it in 28. But if selectrum supports it earlier, I am happy to add support to marginalia before 28 is released.

@clemera
Copy link
Collaborator

clemera commented Dec 16, 2020

As described with marginalia you can now define annotations for all file completions. Support for affixation function was also recently added to Selectrum so as soon marginalia supports them, file icons should also be possible. Can this be closed?

@clemera clemera added the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Dec 16, 2020
@terlar
Copy link
Author

terlar commented Dec 16, 2020

Sounds good, I haven’t checked out affixiation or marginalia yet. But sounds like it will do the trick, thank you!

@terlar terlar closed this as completed Dec 16, 2020
@clemera
Copy link
Collaborator

clemera commented Dec 16, 2020

Affixation function support is not added to marginalia yet but @minad is planning to do so at some point.

@raxod502 raxod502 removed the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Dec 25, 2020
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

5 participants