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.

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

Autoloading #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Autoloading #13

wants to merge 1 commit into from

Conversation

mavit
Copy link
Contributor

@mavit mavit commented Jul 2, 2021

This allows a user to install this package via (list-packages) and start using is straight away, without having to add (require 'flycheck-languagetool) to their ~/.emacs.

This allows a user to install this package via `(list-packages)` and start using is straight away, without having to add `(require 'flycheck-languagetool)` to their `~/.emacs`.
@jcs090218
Copy link
Member

Would this slow the Emacs startup? I might recommend user to add this to their configuration.

(with-eval-after-load 'flycheck (require 'flycheck-languagetool))

@mavit
Copy link
Contributor Author

mavit commented Jul 2, 2021

Would this slow the Emacs startup?

Maybe? It will cause Flycheck to load at startup. However, active Flycheck users are probably already loading it at startup by running, say, (global-flycheck-mode).

I might recommend user to add this to their configuration.

(with-eval-after-load 'flycheck (require 'flycheck-languagetool))

We could do that ourselves in the following way. This seems like a nice solution, but package-lint warns that it’s probably a bad idea.

;;;###autoload
(with-eval-after-load 'flycheck
  (unless (featurep 'flycheck-languagetool)
    (require 'flycheck-languagetool)))

@jcs090218
Copy link
Member

Maybe? It will cause Flycheck to load at startup. However, active Flycheck users are probably already loading it at startup by running, say, (global-flycheck-mode).

I would prefer not to force user to load flycheck at startup. Leave options to users should be a better design of the package. That's said, I can accept with-eval-after-load.

This seems like a nice solution, but package-lint warns that it’s probably a bad idea.

Yeah, try not to have the package-lint warning unless it's necessary. 👍

@mavit
Copy link
Contributor Author

mavit commented Jul 2, 2021

I have asked on StackExchange if anyone knows of a better way of doing this. It seems like it should be quite common.

@jcs090218
Copy link
Member

I know there is function with-no-warnings, not sure if this is a good solution though.

I personally think that autoloading isn't necessary, I would suggest to leave the current code base as it? Users would just have to load it themselves so the program works as they expected to. ;)

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