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

Reader Conditionals #1827

Closed
hypirion opened this issue Feb 10, 2015 · 18 comments
Closed

Reader Conditionals #1827

hypirion opened this issue Feb 10, 2015 · 18 comments

Comments

@hypirion
Copy link
Collaborator

http://dev.clojure.org/display/design/Reader+Conditionals contains information about upcoming changes in Clojure (presumably 1.7). Right now, the only thing that's of importance seems to be supporting reading/compiling .cljc files.

Enabling/disabling feature sets would only require the user to update the :jvm-opts, so there should be no issues there. However, merging these options would be messy (#878 might remedy this problem).

@jenanwise
Copy link

Right now lein completely ignores .cljc files (e.g., lein test will not pick them up) because bultitude does not recognize them. I added partial support in of Raynes/bultitude#27 but leiningen's use of bultitude.core/path-for will have to modified/removed since the 1-to-1 namespace-to-possible-filename assumption is now broken.

@sfnelson
Copy link
Contributor

With Clojure 1.7 nearing release, this issue should be a priority. Reader conditional support is a huge boon for full stack clj/cljs developers, so it would be good to know that leiningen will be ready before 1.7 lands.

@technomancy
Copy link
Owner

Issues become prioritized when someone chooses to work on them. The best way to make sure something happens is to make it happen.

@sfnelson
Copy link
Contributor

#1911

(also Raynes/bultitude#28)

@technomancy
Copy link
Owner

Nice! Could we get some clojurescript users to try this out and leave feedback?

@sfnelson
Copy link
Contributor

@technomancy: there is one function that I have not updated to support cljc that probably needs to be updated, but I don't understand its purpose. I was hoping you could explain what source-in-project? (src/leiningen/compile.clj#L68) is intended to do?

It's only use is in the following function, class-in-project?, where it is called on the parent of a class file. The function itself adds ".clj" to the parent and looks for the result in source dirs:

=> (class-in-project? project /path/to/target/classes/pkg/file.class)
=> (source-in-project? /path/to/target/classes/pkg compile-path source-path)
=> (.exists? /path/to/src/pkg.clj)

Is there a clojure compilation pattern that results in class files created in a subdirectory named after the compiling namespace?

@sfnelson
Copy link
Contributor

sfnelson commented Jun 1, 2015

#1911 is ready for testing now. It's only somewhat relevant to Clojurescript users, it's most relevant to users who previously shared code between Clojurescript and Clojure using CLJX, particularly if they have shared tests or need AOT support.

@technomancy
Copy link
Owner

technomancy commented Jun 1, 2015 via email

@sfnelson
Copy link
Contributor

sfnelson commented Jun 1, 2015

Thanks. I understand the reason to want to clean non-project class files, but I don't understand how the third clause of class-in-project? correctly identifies namespaces for project classes. Given the file target/classes/foo/bar/baz__init.class it looks like the third clause will attempt to resolve src/foo/bar.clj, when I would expect it to attempt to resolve src/foo/bar/baz.clj. In all causes that I've been able to produce it doesn't matter because one of the first two clauses finds the correct project source file.

Is there a case I'm not familiar with where src/foo/bar.clj will generate src/foo/bar/*.class? If not, I think that the third clause is incorrect.

@technomancy
Copy link
Owner

technomancy commented Jun 1, 2015 via email

@sfnelson
Copy link
Contributor

sfnelson commented Jun 1, 2015

Thanks. I've updated the merge request to support this use case.

technomancy added a commit that referenced this issue Jul 25, 2015
Add support for reader conditional files (cljc) (#1827)
@hypirion
Copy link
Collaborator Author

hypirion commented Aug 9, 2015

This is out in 2.5.2. I assume that if we don't hear too much about this not working, then this could be closed.

@sfnelson
Copy link
Contributor

Verified working for crossover code ported from cljx, including an extensive test suite.

@interstar
Copy link

Stupid question, but what do I need to do exactly to get lein to recognise cljc files?

I just renamed a bunch of cljx files to cljc and put them into a directory called cljc/myproject/*cljc

I removed all the cljx plugin references from my project.clj file.

I pulled the latest lein 2.5.2. Did a lein clean and then a lein test and got the compiler blowing up, complaining that it can't find the first of my cljc files.

java.io.FileNotFoundException: Could not locate myproject/maths__init.class or myproject/maths.clj on classpath., compiling:(core.clj:1:1)

Anything else I need to do? Is there a particular place lein is looking for the cljcs?

@danielcompton
Copy link
Collaborator

@interstar is the cljc folder in your :source-paths?. Are you using Clojure 1.7?

@interstar
Copy link

Ah yes, that was it. (No cljc in :source_paths ). I presumably didn't need to explicitly state that when I was using the cljx plugin to put clj files into src so there was nothing explicit. Thanks

@chadharrington
Copy link

This is working for me with lein 2.5.2. I suggest closing the issue.

@hypirion
Copy link
Collaborator Author

hypirion commented Sep 6, 2015

I've tested it to some extent myself with the same results, so I'll close this. Thanks for verifying.

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

No branches or pull requests

7 participants