Comments (14)
I think the problem is that the clojure.lang.LispReader
used in parse*
does not know about any namespace aliases previously read.
See below. My guess is the namespace has to be require'd/use'd for namespace aliases to be recognized by the runtime environment.
(let [source-string "(require '[marginalia.parser :as m])::m/asdf"
make-reader #(java.io.BufferedReader.
(java.io.StringReader. (str source-string "\n")))
lines (vec (line-seq (make-reader)))
reader (clojure.lang.LineNumberingPushbackReader. (make-reader))
old-cmt-rdr (aget (get-field clojure.lang.LispReader :macros nil) (int \;))]
(doall
(take 2
(repeatedly
(fn []
(. clojure.lang.LispReader
(read reader false nil false)))))))
#<CompilerException java.lang.RuntimeException: java.lang.RuntimeException: clojure.lang.LispReader$ReaderException: java.lang.Exception: Invalid token: ::m/asdf (REPL:113)>
from marginalia.
I pushed a potential fix for this. Try it out at http://clojars.org/marginalia (version 0.6.1)
from marginalia.
Keywords in Clojure can be prefixed with any valid symbol. Doing ::foo
just so happens to use the ns as that prefix. Marg was always able to handle the keyword, but it was not dispatching properly during parse.
from marginalia.
AFAICT I still have problems.
This is the culprit for me: https://github.com/swannodette/match/blob/master/test/match/test/core.clj#L314
from marginalia.
WHUT!? ::m/foo is not a legal keyword. I know it works, but still -- that's a bug methinks.
from marginalia.
I've changed to fully qualified keywords for core.match, it works without a hitch.
from marginalia.
Great news. I'll talk to Core about this particular issue. This is edge-case city. :-)
from marginalia.
population: 2
from marginalia.
::m/foo
is valid shorthand for ::match.core/foo
, provided (require '[match.core :as m])
has been specified:
(require '[clojure.set :as set])
::set/foo
; => :clojure.set/foo
The way this happens inside clojure.lang.LispReader
seems quite deliberate -- not likely a bug.
I have a possible fix at the issue-54-namespaced-keywords branch of my fork. The idea is to keep track of the current read-time namespace as forms are being read and evaluate ns
, in-ns
, require
, use
and alias
forms immediately when they are encountered, so that any the read-time environment is set up appropriately before the next form is processed by the reader.
It might have been simpler to require
any namespaces defined by ns
or in-ns
rather than evaluate the namespace-manipulating forms. I decided against this approach partly because of some vague hunches I had, and partly because writing the test was easier that way (no need to create a namespace and install an alias in it before running the actual test). I can provide an alternative patch based on this approach if desired.
from marginalia.
@fogus: Oh, I somehow missed the comment about you planning to talk to others at Core about this. I certainly hope this will not be deemed an undesirable feature, I quite like the shorthand. :-)
from marginalia.
That syntax was definitely new to me, and I was unable to find a reference to it. However, I will roll in your patch as it's generally useful. I did forget to bring it up last week, but I will do so this week. :-) Thank you for the patch. I appreciate your work.
from marginalia.
Great, thanks! This is a fantastic project and I'm really happy to be able to contribute.
from marginalia.
Turns out that attempt at a solution was a bit of a blunder -- if the ns
forms and the like are evaluated, but nothing else is, any :use
clauses / use
calls referring to namespaces which have not already been require
'd by the time Marginalia's parse sees them are guaranteed to refer to non-existing Vars, which leads straight to a crash.
Thus, I committed a reversal of the last patch on my topic branch and came up with a new approach which simply installs a custom reader for keywords (and then uninstalls it after parsing, of course). This is actually more robust, since ::foo/bar
might be valid in some codebase by virtue of a call to use
being made inside a top-level function call, say -- a situation which wouldn't have been handled well by the previous approach even if all the relevant namespaces were loaded.
A more detailed discussion is included in the commit message; and here is a link to the topic branch.
Sorry for the earlier breakage... Here's hoping no similar discoveries w.r.t. the new approach are forthcoming!
from marginalia.
I went ahead and pushed the fix described above; the previous version was known broken, so this can't be any worse, and who knows, it might actually work! ;-)
from marginalia.
Related Issues (20)
- Markdown extension for linking to a function in docstrings and comments HOT 1
- chokes on Clojure 1.7.0 HOT 8
- Marginalia waits a long time for agents to shut down HOT 2
- Every 3rd time I run, I get an exception HOT 3
- Explore the use of Instaparse
- Processing reader conditionals broken HOT 9
- `lein marg` fails when a map value symbol contains a double colon HOT 8
- Incompatibility in matchSymbol with Clojure 1.9.0-beta1 HOT 4
- lein marg fails under clojure 1.9.* HOT 11
- Does not take into account `:source-paths` HOT 1
- Markdown target HOT 4
- :extend-via-metadata in protocols breaks Marginalia HOT 3
- docco url in README.md HOT 1
- Add instructions for `deps.edn` projects HOT 1
- Transfer to clj-commons? HOT 1
- Make [[docstring-links]] into links HOT 1
- [Idea] Respect .gitignore
- Support blocking search engines (support custom <head> content?) HOT 3
- Update the website link in repo's "About" section HOT 1
- [Idea] Ways to style the docs based on metadata on `def`s HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from marginalia.