feat(dired): restructure the module based on dirvish

[alexluigit]

• Deprecate +ranger flag, use dirvish by default

  See: Thank you! alexluigit/dirvish#34

• Bump the package version for diredfl and dirvish

• Reimplement +icons, deprecate package all-the-icons-dired

  all-the-icons-dired is very slow on large directories, while dirvish render icons lazily.

• Use vc-state from dirvish, remove diff-hl-dired-mode

  1. diff-hl-dired-mode doesn't index the entries in subtrees.
  2. It freezes Emacs on big repos, while dirvish does this asynchronously in
     external process and update the UI in an nonblocking fashion.

• Use git-msg from dirvish, deprecate package dired-git-info

  Same reasons as in diff-hl-dired-mode.

• Use utilities in dirvish-yank.el, deprecate package dired-rsync

  dirvish-yank has inbuilt support for rsync.

• Use dirvish-fd, deprecate package fd-dired

  dirvish-fd is way more powerful than fd-dired and integrates flawlessly with dirvish

• Remove the gnuls check on remote hosts

  dirvish checks gnu ls availability for each remote host.

• Set dired-hide-details-hide-symlink-targets to t (default)

  We have symlink target showing on the modeline

• Add a few sensible keybindings globally

• New default dired bindings

Fixes #6562
Replaces #6568

Screenshots:

Before

See: #6568

After

Dired in single window:

Dired in fullscreen (dirvish):

Dired as project sidebar (dirvish-side):

---

• I searched the issue tracker and this hasn't been PRed before.
• My changes are not on the do-not-PR list for this project.
• My commits conform to the git conventions.
• My changes are visual; I've included before and after screenshots.
• I am blindly checking these off.
• Any relevant issues or PRs have been linked to.
• This a draft PR; I need more time to finish it.

[LemonBreezes]

Thank you so much! I have been dying for a proper (dired +dirvish) update and this is so much better. <3

[UndeadKernel]

When it comes to the emacs bindings, they look reasonable to me.

[alexluigit]

@LemonBreezes , no problem! And FYI now the syntax is :emacs dired or :emacs (dired +icons +bindings), no +dirvish required.

[alexluigit]

Hi @UndeadKernel , thanks for your review. I've also included dirvish-side in the latest commit (for those who do not use neotree or treemacs), I hope it makes sense. The screenshot for this feature has been added above.

[UndeadKernel]

The emacs bindings still look good.
I would consider removing the +bindings flag and leave your proposed bindings on by default. This would be in line with Doom's motto of being an opinionated distribution.

[alexluigit]

Thanks. Fixed it as you suggested.

[dschrempf]

Hi! This looks great! Thanks also for focussing on the documentation.

I saw that the module includes dirvish by default. I do not think this is a good idea because there will be users that prefer to use standard dired. Does this module provide any non-dirvish related configuration? If not, can we rename this module to dirvish? If yes, can we make dirvish an optional flag?

What do you think?

[alexluigit]

Hi @dschrempf , thanks for the review.

Does this module provide any non-dirvish related configuration? If not, can we rename this module to dirvish? If yes, can we make dirvish an optional flag?

A main benefit of using dirvish by default is that it rendered these extra packages and their associated hacks obsolete, we only need dirvish and diredfl in packages.el. A optional +dirvish flag (like the current state) actually does not fit in with the module very well.

One can have the classic dired interface by the following settings: (not exactly the same, but very similar)

  (setq dirvish-attributes nil
        dirvish-use-header-line nil
        dirvish-mode-line-format nil
        dirvish-hide-details nil
        dirvish-hide-cursor nil)

I can either add it to the documentation or rename the module to dirvish. I'm okay on both.

[hlissner]

Thanks for the PR! I'm enthusiastic about what it offers, but there are a few issues that must be sorted out first; mainly:

1. To conform to our commit conventions (For example: ce10a12 should be a bump commit, and df986b3 should likely be considered a breaking change).

2. Restore +dired-disable-gnu-ls-flags-maybe-h, as recalculating dired-actual-switches buffer-locally was an intentional effort to support dired over TRAMP, where we can't assume anything about ls (or was changing that meant to imply that dirvish handles this somehow?).

3. To rethink the keybinds. I'm a tad allergic to modules adding more :leader keybinds. More so when they are conditional.

4. Regarding @dschrempf's point. I agree in principle. Enabling dired should produce a reasonably dired-like experience, which is not so much the case with on-by-default dirvish. However, there is a clear benefit to leaving it installed by default, so how about this:

   We keep the +dirvish flag, but its presence will not have any bearing on whether dirvish gets installed or not. Its absence will merely unset the 5 variables you mentioned, and the result is (near-enough) classic dired minus its rough edges. Activating the flag will do the opposite, and you get the dirvish experience, as intended. What do you think?

---

Anyhow, this requires a more careful review, which I'm a bit too swamped to write at this very moment. The above is more to let you (and future Henrik) know that this PR hasn't gone unnoticed, and what's in store for it. In the meantime, I've converted this into a draft while I figure out how best to go about it.

[alexluigit]

Thanks for your review, @hlissner.

1. To conform to our commit conventions (For example: ce10a12 should be a bump commit, and df986b3 should likely be considered a breaking change).

Sorry for that, I've squashed them into a single commit.

2. Restore +dired-disable-gnu-ls-flags-maybe-h, as recalculating dired-actual-switches buffer-locally was an intentional effort to support dired over TRAMP, where we can't assume anything about ls (or was changing that meant to imply that dirvish handles this somehow?).

Yes, it has already handled by dirvish, see: https://github.com/alexluigit/dirvish/blob/132fc6ce95c4d2b491519f5a183bbfb755b34cc8/dirvish-tramp.el#L39-L63

3. To rethink the keybinds. I'm a tad allergic to modules adding more :leader keybinds. More so when they are conditional.

I've reverted all the changes in +emacs/evil-bindings.el. The only thing doesn't look quite right to me is the keybinding for "Open project sidebar (SPC o p)": for now it is mapped when user enabled neotree or treemacs (both are modules turned off by default), but dirvish-side is a viable option in the first place. So I would suggest giving it a keybinding in the future.

4. Regarding @dschrempf's point. I agree in principle. Enabling dired should produce a reasonably dired-like experience, which is not so much the case with on-by-default dirvish. However, there is a clear benefit to leaving it installed by default, so how about this:
We keep the +dirvish flag, but its presence will not have any bearing on whether dirvish gets installed or not. Its absence will merely unset the 5 variables you mentioned, and the result is (near-enough) classic dired minus its rough edges. Activating the flag will do the opposite, and you get the dirvish experience, as intended. What do you think?

Implemented as you described. The line highlighting from dirvish is also removed when +dirvish is not enabled, so that it looks identical to classic dired.

Anyhow, this requires a more careful review, which I'm a bit too swamped to write at this very moment.

Please take your time, and let me know what I can do from my end when you got the chance to review it.

[dararish]

Hi @alexluigit, thank you very much for your work!
Couldn't get dirvish working before, but thanks to you it's perfect with just one minor detail: for some reason after the doom sync, dirvish's side panel was not rendering correctly. It appeared to be running ls with all the flags as if I was using dirvish to view a directory's contents. But once I restarted, all works as expected.

Thank you again :)

[hpfr]

I rebased this PR in hpfr@67d83b8 (https://github.com/hpfr/doom-emacs/tree/dirvish). Some simple conflicts with the doom-package link churn had to be resolved.

I wrote most of what follows a couple months ago but never got around to posting it. To summarize, I think some polishing and tweaks are necessary for a broad (Doom-sized) user base, particularly given Dirvish’s size and scope. Lately it seems like the maintainer hasn’t been around as well, which does not bode well when it comes to the corner cases a Doom-sized user base will encounter.

---

Felt obligated to test this after complaining about crashing in the original PR. With this PR it’s gone 🎉. What follows is a brain dump from daily driving this over the last few days.

Issues:

1. Dirvish loads after Dired, so the first visit to a Dired buffer doesn’t have the Dirvish tweaks
2. If I have a Dirvish session open and open a new project workspace with SPC p p, the file I choose opens in a tiny window at the bottom of the screen, and one of the Dirvish windows takes up the majority
3. The preview for a directory with a .dir-locals.el containing ((nil . ((mode . git-auto-commit)))) fails and shows a backtrace
4. The yank stuff will need evil bindings. y y pasting will be very jarring to evil users
5. Without having visited a pdf to load pdf-tools, the preview warns it’s required. I think Dirvish could just try to load it before giving the warning, no?
6. With the treemacs sidebar open, SPC w c (+workspace/close-window-or-workspace) on the single other window closes the workspace as expected. With dirvish-side, (error Window is dedicated to ‘project-name’) is reported and nothing happens.
7. With +dirvish, simple dired buffers now hide details. The modeline shows the date for the file the cursor is on, but frankly I prefer having the details for the whole directory at a glance by default. For example, I frequently sort by date modified and like being able to scan my eyes down the modified column to see dates for everything at a glance. Personally, if I want a minimal listing, I just peruse the Vertico completion (and even that has details via Marginalia). If I’m opening a buffer, I definitely want the details. Maybe hiding the details makes more sense in a full-frame context where navigation is the focus, but in a non-full frame buffer I definitely want the details right away. dirvish-hide-details applies to dirvish-side, dirvish, and dired all at once, as far as I can tell. Personally, I really only want the details in the non-full-frame, classic (but dirvish-enhanced) dired buffer. That seems like a sane default for Doom, as well.
8. I personally find the whole-line highlight kind of jarring since it appears to use the cursor face by default, and it means you lose the colors in the details. I guess this isn’t a problem for you since you don’t use details in dirvish buffers. But IMO the Doom module should unset dirvish-hide-cursor even when +dirvish is enabled. Doom already has a hl-line feature that is more subtle. Again, this is a thing where I would accept the highlighting in a full-frame dirvish or dirvish-side, but not in a classic dired buffer with details, and I think that’s a pretty common preference.

Ramblings about upstream concerns as opposed to the Doom module

1. The collapsing of single-child directories is cool, but then I expect visiting a line like that to take me straight to the deepest child. I could be in the minority with that, though. I could see how others might find the reverse jarring. Maybe another binding to go straight to the deepest child makes sense? I would suggest S-RET, but that’s taken.
2. Why not make the collapse separator the standard path separator /? | is not a forbidden character in Unix filenames, and it seems like a / would make sense more immediately to new users. I don’t really think | is any more aesthetic, I guess
3. Same but for the separator in the header
4. An audio-only webm gets a little empty box for a preview. Maybe if the video metadata is all nil, treat audio/video container formats like webm and mkv as audio?
5. I like dirvish-use-mode-line and dirvish-use-header-line, but would it be that bad if they were limited to the “center” buffer even in full-frame views? I personally think making tiny windows to span the main windows for the header and mode line is not worth the complexity. It doesn’t gain you that many characters and costs you a couple extra lines.
6. I like having the default find-file action on a directory open a simple dired buffer, but an embark action for dirvish would be cool
7. The quick access and history features are cool, but it feels weird that they’re Dirvish-specific. It doesn’t compose well and feels out of scope, I guess. Personally, I’d rather just have bindings to consult-bookmark and consult-dir than to have to manually configure another set of bookmarks or mentally bother with whether I visited a directory in Dirvish specifically.
8. The built-in Zip-Archive mode can preview zip contents without zipinfo via archive-zip-summarize. maybe leverage that for zip preview as a fallback, or first if it’s better than zipinfo?
9. Fallback to bsdtar -tvf for preview if unzip is not present would also be cool
10. The name dirvish-hide-cursor isn’t particularly indicative of the functionality; something like dirvish-hl-line would be clearer in my opinion.
11. Emerge is an Emacs package and the primary package management command for Gentoo. It’s a clever name for a pinning tool, but something less creative dirvish-pin might be more intuitive?

I don’t think Henrik tried the module when he made his +dirvish behavior suggestion in #6760 (review). Having tried it, I think only dirvish-hide-details and dirvish-hide-cursor are jarring enough to unset by default. The header line and mode line are clear-cut improvements for standard dired buffers. I personally doubt the attributes (including collapse) are going to confuse people who update unaware of the backend change; they also seem like improvements rather than matters of taste. If we’re down to two variables, I don’t even think the +dirvish flag is necessary, and the module README can just point out setting dirvish-hide-details and dirvish-hide-cursor as the path to matching upstream Dirvish appearance, and users can just enable those two options at their preference. That said, I think addressing 7 and 8 from the first list would really cement this approach. I’ll leave deciding on the defaults up to Henrik, of course, I just figured I’d provide this point of view in case it’s useful.

I guess this probably comes off as very critical since I’m just dumping places I’m tripping over, but to be clear, I can already tell this is an incredible labor of love and I’ve noticed lots of nice details even without playing with some of the more powerful features yet, so thanks very much for your time and effort. Brilliant work.

[LemonBreezes]

For 1., I wrote my own hack which works:

(add-hook 'find-directory-functions
          (defun cae-dired-load-dirvish-h (dir)
            (remove-hook 'find-directory-functions #'cae-dired-load-dirvish-h)
            (require 'dirvish nil t)
            (unless (memq #'dired-noselect find-directory-functions)
              (add-hook 'find-directory-functions #'dired-noselect t))
            (dired-noselect dir))
          t)

2. I also find this very annoying and have tried to fix it myself but haven't come up with one. Advising find-file (for me) leads to a lot of bugs in edge cases, etc.
3. I also get an error for this, though mine may be different from yours:
   
4. I also can't close the main window in this scenario. This is correct, however +workspace/close-window-or-workspace should delete the workspace. But +workspace/close-window-or-workspace doesn't handle having windows with the no-other-window property that are not popup windows as they are returned by doom-visible-windows.
5. I agree. I want details Dirvish whenever I'm not using the fancy fullscreen layout.
6. I also agree here. I also don't like how the details colors get removed by the highlighting.

[hpfr]

Thanks for the confirmations, but there’s really no need to quote the entire diatribe when your reply directly follows 😅

[hlissner]

Sorry for the tremendous wait! I plan to test this PR this weekend, formulate a proper review, and address the points in @hpfr's post. I'm a bit out of the loop, so if there are any blockers or pressing concerns with the PR's current state or upstream packages (besides what's already been mentioned), please let me know!

[hpfr]

Lately it seems like the maintainer hasn’t been around as well, which does not bode well when it comes to the corner cases a Doom-sized user base will encounter.

I guess I already mentioned this, but I’ll call it out here just in case. I don’t know if you want to go to the trouble of reviewing given that the maintainer looks to be completely inactive in recent months, unless you are willing to maintain a fork of the package.

[hpfr]

Well, if you still want to take a look, I rebased once more on master at https://github.com/hpfr/doom-emacs/tree/dirvish. I had to resolve the nerd-icons swap, but I don’t currently use icons so it’s untested. I also bumped the module packages

Edit: One more rebase resolving nerd icons docs replacement and nerd-icons-dired bump

[pharcosyle]

Rebased this again on the latest Doom master: https://github.com/pharcosyle/doomemacs/tree/dirvish