Skip to content

Text icons#38

Open
MArpogaus wants to merge 27 commits intochaosemer:text-iconsfrom
MArpogaus:text-icons
Open

Text icons#38
MArpogaus wants to merge 27 commits intochaosemer:text-iconsfrom
MArpogaus:text-icons

Conversation

@MArpogaus
Copy link

@MArpogaus MArpogaus commented Apr 27, 2025

Hi @chaosemer,

this PR addresses my feature request for unicode icons in #37.
I just took some time to hack around a bit to get visual appealing toolbar in the mode line using nerd icons:

image

Also works in terminal if the correct font is configured.

image

I used your code in the text-icons branch and merged the latest changes from main.
I also fixed the function window-tool-bar--find-unicode-icon to work robustly with different key maps.
I currently return "?" if no icon is defined, returning the icon name instead might be more helpful.
Additionally, separating a lists per key map could be beneficial if icon names collide.

However, images, don't seam to work anymore with the current version.

This is my setup if you want to reproduce is:

(use-package window-tool-bar
    :ensure (:host github :repo "MArpogaus/window-tool-bar" :branch "text-icons" :files ("window-tool-bar.el" (:exclude ".git")))
    :custom
    (window-tool-bar-unicode-image-map '(;; Prefer text in CP437 for display compatibility.
        (new "")
        (open "")
        (diropen "")
        (close "󱎘")
        (save "")
        (undo "󰕌")
        (cut "")
        (copy "")
        (paste "󰆒")
        (search "")
        (help "󰘥")
        (index "󰋽")
        (search-replace "󰛔")
        (exit "󰸞")
        (right-arrow "")
        (left-arrow "")
        (next-node "")
        (prev-node "")
        (up-node "")
        (home "")
        (jump-to "󰑎")))
    :custom-face
    (window-tool-bar-button-disabled ((t :background unspecified :inherit window-tool-bar-button))))

I know that i should not use nerd icons directly, but rather obtain them by name.
The package should contain some sane defaults for default tool bars.
For this to work you need to use a patch nerd font global.

There is much room for improvement but at least i we seam to have a first working version here.

I'm looking forward to your feedback

chaosemer and others added 24 commits February 8, 2025 07:01
The window tool bar did not show buttons with a label set to "". Use the symbol name of the binding in this case.
This fixes the error with the back-button package (https://github.com/rolandwalker/back-button) which defines bindings with the shift modifier.
This check breaks on core ELPA packages and packages that use compat.
@chaosemer
Copy link
Owner

I appreciate the extra commits. This is something I've wanted to do for a bit but have been distracted by other things.

Since this file is part of Gnu Emacs, any contributions to it need to be by people that have followed Emacs' contribution steps, especially the Copyright Assignment steps outlined here. Have you done so?

@MArpogaus
Copy link
Author

No, but I can surely look into it.

Is this also important during development, or can we continue working without until we finally merge into emacs?

@chaosemer
Copy link
Owner

I'd like to start the Emacs contribution process first. We can do this in parallel to reviewing the code. I sent an email to emacs-devel and CC'd your @htwg-konstanz.de email.

I'll look over this pull request next.

@chaosemer
Copy link
Owner

Some thoughts:

The change from fix: add keybindings for mode-line (e1c258c) looks great and it might be easier if that was its own pull request. Up to you.

I think it may make more sense for unicode-image to be a separate user option that controls whether to use unicode icons or image icons. That way if a buffer is displayed in a graphical terminal the image icons can be used and if the buffer is displayed in a text-only terminal the unicode icons would be displayed. I imagine some people would want to force unicode icons for graphical displays if they've done significant customization.

@MArpogaus
Copy link
Author

Sure, just created a new PR #39 for the bindings.

@MArpogaus
Copy link
Author

I'd like to start the Emacs contribution process first. We can do this in parallel to reviewing the code. I sent an email to emacs-devel and CC'd your @htwg-konstanz.de email.

I'll look over this pull request next.

Great, thank you. I'll look into this tomorrow

@MArpogaus
Copy link
Author

MArpogaus commented May 4, 2025

I think it may make more sense for unicode-image to be a separate user option that controls whether to use unicode icons or image icons. That way if a buffer is displayed in a graphical terminal the image icons can be used and if the buffer is displayed in a text-only terminal the unicode icons would be displayed. I imagine some people would want to force unicode icons for graphical displays if they've done significant customization.

Actually this was my intended behaviour. As long as window-tool-bar-style is not customized to unicode-image, regular images are used in graphical frames. Is this not working for you?

@chaosemer
Copy link
Owner

I was hoping that when window-tool-bar-style is set to image the unicode icons would be used in text frames and the images are used in graphical frames. (This is a good behavior change!) My understanding of this patch is that when set to image, images are always used. Am I missing something?

I'm torn if it makes more sense to add the value unicode-image to window-tool-bar-style or if introducing a new user option window-tool-bar-image-display-style that would select between images and unicode icons is preferred. I guess it's nice to not have to introduce a new user option if we can avoid it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants