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.

Sign up for GitHub

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

feat: add all-the-icons support #5

Closed
wants to merge 5 commits into from
+59 −14
Closed

feat: add all-the-icons support #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47661bc
Add all-the-icons backend support
librephoenix Jun 25, 2024
0086005
Update README indicating all-the-icons support
librephoenix Jun 25, 2024
7a21d1f
Comment for optional argument allowing selection of icon backend for nix
librephoenix Jun 25, 2024
49e62a2
Refresh icon backend automatically and allow custom icon getter funct…
librephoenix Jun 25, 2024
d2fae0b
Revert patch-template additions and fixes icon backend refreshing
librephoenix Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ For example, with `use-package`:
If you are using some other method to install, you will need to ensure the following dependencies:

- `el-patch`
- `nerd-icons`
- `nerd-icons` or `all-the-icons`
- `magit`

## Nix
Expand Down Expand Up @@ -87,9 +87,7 @@ A minimal flake for creating an Emacs with the `magit-file-icons` package could

# Commentary

This package uses [nerd-icons.el](https://github.com/rainstormstudio/nerd-icons.el) to render icons. Currently, this is the
only supported icon backend.
This package uses [nerd-icons.el](https://github.com/rainstormstudio/nerd-icons.el) by default to render icons, but if nerd-icons is not found, it will attempt to use [all-the-icons.el](https://github.com/domtronn/all-the-icons.el) instead. Currently, these are the only supported backends.

The author is not opposed to adding additional icon backends — such as [all-the-icons.el](https://github.com/domtronn/all-the-icons.el)
or [vscode-icons-emacs](https://github.com/jojojames/vscode-icon-emacs) — in the future.
The author is not opposed to adding additional icon backends — such as [vscode-icons-emacs](https://github.com/jojojames/vscode-icon-emacs) — in the future.

64 changes: 55 additions & 9 deletions magit-file-icons.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@

;; Places icons next to file entries in Magit buffers based on file extension.
;; This is intended to improve visual clarity and ease of gleaning information.
;; Currently, only the `nerd-icons' backend is supported.
;; Currently, the `nerd-icons' backend is the default, but it will fallback
;; to `all-the-icons' if nerd-icons is not found.

;;; Code:

(require 'el-patch)
(require 'el-patch-template)
(require 'magit)
(require 'nerd-icons)

(defgroup magit-file-icons nil
"Show file icons in Magit buffers."
Expand All @@ -55,12 +55,52 @@
:type 'boolean
:group 'magit-file-icons)

(defcustom magit-file-icons-icon-backend 'nerd-icons
"Icon backend for magit-file-icons. Set this to nil
if you want to customize `magit-file-icons-icon-for-file-func'
or `magit-file-icons-icon-for-dir-func'."
:type 'symbol
:group 'magit-file-icons)

(if (not (require 'nerd-icons nil t)) (funcall
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this check is necessary if we have nerd-icons in the Package-Requires clause

(lambda ()
(require 'all-the-icons)
(setq magit-file-icons-icon-backend 'all-the-icons))))

(defcustom magit-file-icons-icon-for-file-func nil
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use :type 'function? It's a perfectly valid defcustom type, and means you don't have to explicitly explain how to customize the variable in the docstring.

"Icon for file function. Automatically set if
`magit-file-icons-icon-backend' is non-nil.
Customize using (fset 'magit-file-icons-icon-for-file-func 'custom-function)."
:type 'symbol
:group 'magit-file-icons)

(defcustom magit-file-icons-icon-for-dir-func nil
"Icon for directory function. Automatically set if
`magit-file-icons-icon-backend' is non-nil.
Customize using (fset 'magit-file-icons-icon-for-file-func 'custom-function)."
:type 'symbol
:group 'magit-file-icons)

(defun magit-file-icons-refresh-backend ()
"Refresh backend according to `magit-file-icons-icon-backend'.
Does not refresh if `magit-file-icons-icon-backend' is nil."
(if magit-file-icons-icon-backend
(if (eq magit-file-icons-icon-backend 'nerd-icons)
(funcall (lambda ()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't mutate customizable variables - these are only meant to be touched by the user. Instead, define an internal variable and derive its initial state from any values for custom variables set by the user.

(fset 'magit-file-icons-icon-for-file-func 'nerd-icons-icon-for-file)
(fset 'magit-file-icons-icon-for-dir-func 'nerd-icons-icon-for-dir)))
(funcall (lambda ()
(fset 'magit-file-icons-icon-for-file-func 'all-the-icons-icon-for-file)
(fset 'magit-file-icons-icon-for-dir-func 'all-the-icons-icon-for-dir))))))

(magit-file-icons-refresh-backend)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can get rid of this line if we call the refresh function in the minor-mode anyway

Suggested change
(magit-file-icons-refresh-backend)


(el-patch-define-template
(defun magit-diff-insert-file-section)
(format (el-patch-swap "%-10s %s" "%-10s %s %s") status (el-patch-add (nerd-icons-icon-for-file (or orig file)))
(format (el-patch-swap "%-10s %s" "%-10s %s %s") status (el-patch-add (magit-file-icons-icon-for-file-func (or orig file)))
(if (or (not orig) (equal orig file))
file
(format (el-patch-swap "%s -> %s" "%s -> %s %s") orig (el-patch-add (nerd-icons-icon-for-file file)) file))))
(format (el-patch-swap "%s -> %s" "%s -> %s %s") orig (el-patch-add (magit-file-icons-icon-for-file-func file)) file))))

(el-patch-define-template
(defun magit-insert-untracked-files)
Expand All @@ -69,15 +109,15 @@
(el-patch-swap file
(format "%s %s"
(if (file-directory-p file)
(nerd-icons-icon-for-dir file)
(nerd-icons-icon-for-file file))
(magit-file-icons-icon-for-dir-func file)
(magit-file-icons-icon-for-file-func file))
file))
'font-lock-face 'magit-filename)
?\n))

(el-patch-define-template
(defun magit-diff-wash-diffstat)
(insert (propertize (el-patch-swap file (format "%s %s" (nerd-icons-icon-for-file file) file)) 'font-lock-face 'magit-filename)
(insert (propertize (el-patch-swap file (format "%s %s" (magit-file-icons-icon-for-file-func file) file)) 'font-lock-face 'magit-filename)
sep cnt " "))

;;;###autoload
Expand All @@ -88,11 +128,17 @@
(magit-file-icons-mode
(when magit-file-icons-enable-diff-file-section-icons (el-patch-eval-template #'magit-diff-insert-file-section 'defun))
(when magit-file-icons-enable-untracked-icons (el-patch-eval-template #'magit-insert-untracked-files 'defun))
(when magit-file-icons-enable-diffstat-icons (el-patch-eval-template #'magit-diff-wash-diffstat 'defun)))
(when magit-file-icons-enable-diffstat-icons (el-patch-eval-template #'magit-diff-wash-diffstat 'defun))
(when magit-file-icons-enable-diff-file-section-icons (magit-file-icons-refresh-backend))
(when magit-file-icons-enable-untracked-icons (magit-file-icons-refresh-backend))
(when magit-file-icons-enable-diffstat-icons (magit-file-icons-refresh-backend)))
Comment on lines +132 to +134
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to combine these into an or clause, or just run the refresh unconditionally

Suggested change
(when magit-file-icons-enable-diff-file-section-icons (magit-file-icons-refresh-backend))
(when magit-file-icons-enable-untracked-icons (magit-file-icons-refresh-backend))
(when magit-file-icons-enable-diffstat-icons (magit-file-icons-refresh-backend)))
(magit-file-icons-refresh-backend)

(add-hook 'magit-mode-hook 'magit-file-icons-refresh-backend)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can get rid of these hooks - it's probably sufficient to refresh on minor mode activation.

('deactivate
(el-patch-unpatch #'magit-diff-insert-file-section 'defun nil)
(el-patch-unpatch #'magit-insert-untracked-files 'defun nil)
(el-patch-unpatch #'magit-diff-wash-diffstat 'defun nil))))
(el-patch-unpatch #'magit-diff-wash-diffstat 'defun nil)))
(remove-hook 'magit-mode-hook 'magit-file-icons-refresh-backend))


(provide 'magit-file-icons)

Expand Down
1 change: 1 addition & 0 deletions nix/packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# TODO add optional argument specifying icon backend (nerd-icons or all-the-icons) and modify packageRequires accordingly
_: {
perSystem =
{ lib, pkgs, ... }:
Expand Down