domtronn / all-the-icons.el

A utility package to collect various Icon Fonts and propertize them within Emacs.
MIT License
1.48k stars 177 forks source link

Check if fonts already installed #120

Open guilherme-salome opened 6 years ago

guilherme-salome commented 6 years ago

Hi, I am using use-package to install my packages automatically, and I was wondering if it is possible to check if the fonts are already installed, and if they are not then install them.

What I have now is this:

(use-package all-the-icons
  :ensure t
  :config (all-the-icons-install-fonts t))

But whenever I run it, I get prompted to install the fonts again. Is it possible to add an if statement there that would check if the fonts are already there? Thanks!

holocronweaver commented 6 years ago

Looking for this exact same feature. Possibly all-the-icons could set a persistent variable indicating fonts have been successfully installed at least once?

guilherme-salome commented 6 years ago

Maybe it's possible to write a function to check where the fonts are installed and see if they are there. I just don't know how to.

shwaka commented 6 years ago

I also came up with the same issue, and wrote the following code in the :config section of use-package. It seems to work well for me (Ubuntu 16.04, Emacs 24.5.1), and I hope it works also for you.

Note that the definition of the local variable font-dest is copied from the source code of the function all-the-icons-install-fonts.

(let ((font-dest (cl-case window-system
                   (x  (concat (or (getenv "XDG_DATA_HOME")            ;; Default Linux install directories
                                   (concat (getenv "HOME") "/.local/share"))
                               "/fonts/"))
                   (mac (concat (getenv "HOME") "/Library/Fonts/" ))
                   (ns (concat (getenv "HOME") "/Library/Fonts/" )))))
  (unless (file-exists-p (concat font-dest "all-the-icons.ttf"))
    (all-the-icons-install-fonts t)))
wyuenho commented 5 years ago

You only ever need to install the font once for as long as you can resist doing a clean install of your OS, why bother checking at all every time you start emacs? Just install it once and forget about it.

holocronweaver commented 5 years ago

It's a single state variable and a single if statement; performance is not a concern. But convenience and user friendliness is! Please re-open.

wyuenho commented 5 years ago

@holocronweaver Does this work okay?

noctuid commented 5 years ago

Would be nice if there was also an option to exit if the fonts are already installed without prompting.

wyuenho commented 5 years ago

@noctuid that's what this PR does, see if it works for you?

noctuid commented 5 years ago

(all-the-icons-install-fonts) in that PR will prompt to continue if the fonts are already installed and (all-the-icons-install-fonts t) will reinstall them no matter what. What I'd like is an option to both not prompt and not reinstall the fonts if they are already installed.

I think it would be nice to throw this in the init file to prevent the need for manual installation. Even ignoring the reinstall situation, there are some other situations where I think this would be useful (e.g. putting your config on a new computer and/or new OS or for use by Emacs starter kits, so the user doesn't have to do any extra setup themselves on first run). The check is quite fast, so it isn't an issue to do it every time Emacs starts.

holocronweaver commented 5 years ago

I think if you simply change:

    (when (or pfx
              (not fonts-installed)
              (and fonts-installed
                   (yes-or-no-p "This will download and install the fonts, are you sure? ")))

to

    (when (or pfx
              (not fonts-installed))

then this solves the issue. I think requiring the prefix to always install is fine, and make the default only to install fonts if they are missing.

wyuenho commented 5 years ago

Okay, I think there are 2 conflicting use cases here. One is to install the fonts automatically without prompting if they don't look like installed, another is to optionally suppress interactivity if they are installed for scripting. Since there's a read-directory-name interactive call there for systems where the installation location is unknown, it's not clear to me how to retrofit this function to be completely noninteractive. My PR has solved the first problem, the second problem may demand some refactoring or a second function.

wyuenho commented 5 years ago

For now, I believe you should be able to stub out yes-or-no-p temporarily with letf in your config block.

noctuid commented 5 years ago

Force reinstalling doesn't seem like a common use case to me.

or systems where the installation location is unknown, it's not clear to me how to retrofit this function to be completely noninteractive

An optional argument could be added to specify the font-path, and if it isn't specified, just do nothing if pfx is non-nil (maybe output a warning).

wyuenho commented 5 years ago

The need seems to be installing the fonts during package installation. I don't think there's a way to eliminate interactivity altogether, it might make sense to just call (all-the-icons-install-fonts) before (provide 'all-the-icons). This will eliminate the need to all (all-the-icons-install-fonts) in use package's config.

seagle0128 commented 5 years ago

My workaround for reference:

(unless (member "all-the-icons" (font-family-list))
    (all-the-icons-install-fonts t))
andreyorst commented 5 years ago

The need seems to be installing the fonts during package installation. I don't think there's a way to eliminate interactivity altogether, it might make sense to just call (all-the-icons-install-fonts) before (provide 'all-the-icons). This will eliminate the need to all (all-the-icons-install-fonts) in use package's config.

what about safety? Installation of all-the-icons package will modify user's system configuration. Isn't this a potential way to attack?

I guess @seagle0128 has a nice workaround for this feature. It's clear that the user wants to install fonts automatically when fonts not installed, and it's quite explicit, not hided inside package code, so expected to happen.

odinu commented 4 years ago

My workaround for reference:

(unless (member "all-the-icons" (font-family-list))
    (all-the-icons-install-fonts t))

@seagle0128 That seems like a great workaround but it fails for me: error: Error reading from stdin Any tips on getting this to work?

andreyorst commented 4 years ago

I check for fonts like this:

(defun aorst/font-installed-p (font-name)
  "Check if font with FONT-NAME is available."
  (if (find-font (font-spec :name font-name))
      t
    nil))
(use-package all-the-icons
  :config
  (when (and (not (aorst/font-installed-p "all-the-icons"))
             (window-system))
    (all-the-icons-install-fonts t)))
seagle0128 commented 4 years ago

@dncrash Did you run M-x all-the-icons-install-fonts manually? I think it's due to the network issue.

@andreyorst Both workarounds work well, but I have to say find-font is more efficient. Thank you!

odinu commented 4 years ago

@dncrash Did you run M-x all-the-icons-install-fonts manually? I think it's due to the network issue.

I did. That works and my icons are now installed and working properly. However I would like to have it in my config so it runs automatically next time I reinstall my OS or something. Running it manually every time seems like such a chore and by then I'll forget what I have to do and why my fonts/icons are missing :)

I tried putting your workaround in the :config section of use-package, and more recently after seeing one of your code snippets, in the :init section

(use-package all-the-icons
    :ensure t
    :init
    (unless (member "all-the-icons" (font-family-list))
    (all-the-icons-install-fonts t)))

And the error I'm getting is:

Error (use-package): all-the-icons/:init: Error reading from stdin