search

wyuenho / all-the-icons-dired

Adds dired support to all-the-icons
GNU General Public License v3.0
23 stars 7 forks source link

Use text properties instead of overlays #17

Closed aikrahguzar closed 2 years ago

aikrahguzar commented 2 years ago

@wyuenho, @protesilaos

A pr to use text properties instead of overlays for the icons as discussed in #16

Results in some performance improvement but I think simplifies the code too. It also fixes #13 dired-icons

It is a pretty invasive change so requires some testing but I have been using it for a while and everything seem ok.

protesilaos commented 2 years ago

Thank you! I wrote some minor comments.

aikrahguzar commented 2 years ago

Thank you! I wrote some minor comments.

Thanks! I mostly applied the changes you suggested!

protesilaos commented 2 years ago

It was all fine, anyway. I appreciate your contribution!

aikrahguzar commented 2 years ago

This PR will insert 2 icons after toggling dired-hide-dotfiles-mode. Have you tested this implementation on all the functions that all-the-icons-dired--refresh-advice advices?

I did test this with all the functions except the dired-narrow one which I didn't have installed. I didn't have dired-hide-dotfiles-mode either but is it this repository: https://github.com/mattiasb/dired-hide-dotfiles ? I tried it just now and I see only one icon. And it uses internally dired-do-kill-lines which also work fine, so I don't see why it will result in two icons.

wyuenho commented 2 years ago

Thank you!