Skip to content

The proper place for core.typed #58

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

Open
bsless opened this issue Jul 6, 2019 · 7 comments
Open

The proper place for core.typed #58

bsless opened this issue Jul 6, 2019 · 7 comments

Comments

@bsless
Copy link

bsless commented Jul 6, 2019

Hello,
I notice there have been several issues opened due to the same root cause - that core.typed was not being required in the project's dependencies. I stumble on this issue and forget to add it myself at times.
Where do you think is the proper place to resolve this issue?
As far as I understand it, even with the type checks disabled, squiggly is still broken without requiring core.typed?
The possible solutions I see are:

  • Add a warning to require it in the README
  • Require in a profile (should also be specified in the readme)
  • inject the dependency

Thank you for the work you do on this project
Ben

@pnf
Copy link
Contributor

pnf commented Jul 7, 2019

I think the real problem is that kibit has crypto-dependency (i.e. one that does not show up in lein deps :tree on core.tools/reader. You can see that this is the problem by turning on nrepl-log-messages and observing a FileNotFoundException. That library used to be provided via dependency on it by cider itself and, as a backup, by core.typed. To avoid a conflict with cider, we had a line around line 203 of flycheck-clojure.el to explicitly exclude the dependency during jack-in. It appears that cider no longer depends on thus provides reader, but you can still get it as a side-effect of declaring a dependency on core.typed. That is of course silly.
Check out PR #59, which drops the exclusion from the elisp injection, adds an explicit dependency on reader to the plugin, and comments-out all the typed references from the sample project. If you could give this a try, I'd really appreciate it.

@bsless
Copy link
Author

bsless commented Jul 8, 2019

I'll gladly give it a shot and report the results back.
Thanks!

@bsless
Copy link
Author

bsless commented Jul 10, 2019

Hi, I tested it in Clojure 1.8 and got

Caused by: java.io.FileNotFoundException: Could not locate clojure/tools/reader/impl/ExceptionInfo__init.class or clojure/tools/reader/impl/ExceptionInfo.clj on classpath.

I also tried adding the reader as a dependency during jack-in and to the project. neither worked. Any ideas?

@pnf
Copy link
Contributor

pnf commented Jul 10, 2019

Odd.
You’d need to do a local install of the unexclude-tools-reader branch to pick up the new explicit dependency, and of course you need to be using the new flycheck-clojure.el that doesn’t exclude during the injection.
Can you verify that you’re picking up the rebuilt local install off the new branch and are using the new .el as well? Then can you paste the full jack-in message?

@bsless
Copy link
Author

bsless commented Jul 11, 2019

I'll recreate it, but I verified that it worked for me as intended by the following steps:

  • open new emacs instance.
  • don't open any clojure file.
  • open new flycheck-clojure.el version from branch and eval it.
  • then open clojure file and jack-in.
    Jack in message:
$HOME/.local/bin/lein update-in :dependencies conj \[acyclic/squiggly-clojure\ \"0.1.9-SNAPSHOT\"\] -- update-in :dependencies conj \[nrepl\ \"0.6.0\"\] -- update-in :plugins conj \[refactor-nrepl\ \"2.4.0\"\] -- update-in :plugins conj \[cider/cider-nrepl\ \"0.22.0-beta1\"\] -- repl :headless :host localhost

Also manually tried this server startup outside of emacs:

$HOME/.local/bin/lein update-in :dependencies conj \[org.clojure/tools.reader\ \"1.3.2\"\] -- update-in :dependencies conj \[acyclic/squiggly-clojure\ \"0.1.9-SNAPSHOT\"\] -- update-in :dependencies conj \[nrepl\ \"0.6.0\"\] -- update-in :plugins conj \[refactor-nrepl\ \"2.4.0\"\] -- update-in :plugins conj \[cider/cider-nrepl\ \"0.22.0-beta1\"\] -- repl :headless :host localhost

@pnf
Copy link
Contributor

pnf commented Jul 11, 2019

And you’re sure it’s getting the snapshot from your local install off of the unexclude-tools-reader branch?

@bsless
Copy link
Author

bsless commented Aug 14, 2019

Hi, sorry I disappeared for a while, work got a bit too intensive,
I verified the way I tested before, and I tested again and couldn't recreate the behavior. Looks like it works for me now. I tried testing and verifying through and through and it looks like I was either doing something wrong previously or had created some state on my system which I couldn't recreate.

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

2 participants