search

Fuco1 / dired-hacks

Collection of useful dired additions
GNU General Public License v3.0
877 stars 77 forks source link

Fixes and adjustments to dired-collapse dependencies #211

Closed Boruch-Baum closed 4 months ago

Boruch-Baum commented 8 months ago

The 'fix' in this PR is to require the s- package, which is used but not declared.

The 'adjustments' in this PR are to remove several needless dependencies.

Boruch-Baum commented 8 months ago

Add me as a new data-point for 'people', I guess. 1) I'm only interested in dired-collapse; 2) My Emacs doesn't have package 'f' installed even though my Emacs is quite 'heavy'; 3) I'm biased against having unnecesary 'stuff' on my machines.

On 2024-03-21 05:28, Matus Goljer wrote:

@Fuco1 commented on this pull request.

In [1]dired-collapse.el:

                         (= 1 (length files)))

(setq path (car files))) (if (and (not files)

  • (equal path (dired-utils-get-filename)))
  • (equal path (dired-get-filename nil t)))

TBH I created the "utils" function to avoid these nil t arguments everywhere. To me it seems like quite a bad API and just a bit confusing passing some boolean flags.

People usually install multiple dired-hacks packages and the utils package will be pulled in 99% of the cases, so I think it's "worth it" to keep it.

f is also present in virtually every user's Emacs since it's depended upon in so many packages. In general I'm not a huge fan of "removing dependencies" just for the sake of not having them. I agree that dash here is not necessary because the built-in performs identical function.

— Reply to this email directly, [2]view it on GitHub, or [3]unsubscribe. You are receiving this because you authored the thread. Message ID: @.***>

References

  1. https://github.com/Fuco1/dired-hacks/pull/211#discussion_r1533813650
  2. https://github.com/Fuco1/dired-hacks/pull/211#pullrequestreview-1952070073
  3. https://github.com/notifications/unsubscribe-auth/AAOE3KB7PE7DFNSJVNZDP4DYZLG7LAVCNFSM6AAAAABEQQIQDWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNJSGA3TAMBXGM

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

Fuco1 commented 4 months ago

I cherry picked the commits removing dash as a dependency and adding require to s. I would prefer to keep the f dependency and dired-hacks-utils. The utils package is a part of dired hacks and the only reason it is distributed separately is because of historical circumstance. I prefer to use consistent api over all the packages.

Similarly with f, I prefer the convenience and readability.

Thanks for the contribution!