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

dependency flycheck is not handled right #32

Open
jian-lin opened this issue Mar 10, 2025 · 4 comments
Open

dependency flycheck is not handled right #32

jian-lin opened this issue Mar 10, 2025 · 4 comments

Comments

@jian-lin
Copy link

It looks like an optional dependency. If so, it should not be used with (require 'flycheck). Otherwise, installing it from MELPA triggers errors. Steps to reproduce:

  1. emacs -Q
  2. eval this
(progn
  (require 'package)
  (add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/") t)
  (package-install 'ocaml-eglot))
  1. see the error in *Compile-Log* buffer
Compiling file /tmp/tmp.rRpW5PPdT2/.emacs.d/elpa/ocaml-eglot-20250309.2027/ocaml-eglot.el at Mon Mar 10 12:24:48 2025
ocaml-eglot.el:37:11: Error: Cannot open load file: No such file or directory, flycheck

One way to avoid that error is to change (require 'flycheck) to (require 'flycheck nil t).

@bbatsov
Copy link
Contributor

bbatsov commented Mar 10, 2025

Yeah, I can commented about this when I saw #29. Given there's an explicit config option for using it, I don't think (require 'flycheck nil t) alone is enough, as someone could select the option would without having installed it, so probably some check and things will just blow up - I personally prefer feature of function checks over conditional requires that might fail at the top-level.

@xvw
Copy link
Member

xvw commented Mar 10, 2025

Looking to: #33
@bbatsov You seem to think that's not enough. Is there any way you could improve? For example, exposing the flycheck option only if it's installed?
Can we merge for the time being, fixing the installation from MELPA, and find a better approach?
Thank you all for your feedback and help, it's very valuable!

@bbatsov
Copy link
Contributor

bbatsov commented Mar 10, 2025

Yeah, you can merge #33 - it will solve the immediate problem.

@xvw
Copy link
Member

xvw commented Mar 10, 2025

Thank you! I'll leave it open to come up with something more “standard”. Thank you very much for your help!

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

No branches or pull requests

3 participants