_Comment made by: zcaudate_
Please reconsider this `fix` for the following reasons:
- (keyword "foo/bar/baz") will also need to be fixed for consistency reasons
- It will break users of my code
-
http://docs.caudate.me/adi/adi-guide.html
-
http://docs.caudate.me/adi/adi-walkthrough.html#schoolyard
Please see discussion on this topic below:
dm3 [5:04 PM]
Is there a reason why `(read-string "a/b/c")` is OK, while `(clojure.tools.reader/read-string "a/b/c")` fails with `Invalid token`?
hiredman [5:04 PM]
there is a ticket to fix read-string
dm3 [5:05 PM]
so the correct behaviour is to fail?
hiredman [5:05 PM]
http://dev.clojure.org/jira/browse/CLJ-1530
dm3 [5:06 PM]
thanks, seems like a breaking change :simple_smile:
hiredman [5:07 PM]
the docs have precluded symbols like a/b/c for some time, and the behavior of how those unspecified symbols were read apparently changed at some point
lucas [5:09 PM]
joined #clojure
zcaudate [5:16 PM]
@hiredman: seriously. I’m really peeved about that because I’ve been using `:a/b/c` keywords for a while… even wrote a whole freaking library to deal with that
http://docs.caudate.me/hara/hara-string.html#api---path (edited)
[5:16]
and now they are taking it out
[5:18]
i’m of the opinion that the `::foo/baz` keywords should be taken out first
[5:20]
```user=> (require '[clojure.walk :as walk])
nil
user=> ::walk/hello
:clojure.walk/hello
```
[5:20]
that causes way more problems
[5:20]
especially with analysers
[5:22]
https://github.com/jonase/kibit/issues/14
GitHub
Kibit breaks on namespace qualified keywords · Issue #14 · jonase/kibit · GitHub
If the code contains namespace qualified keywords with aliases, Kibit errors out with a Invalid token exception. The following code demonstrates the problem - ;;; foo.clj (ns foo) ;;; bar.clj (n...
[5:25]
@dm3 if there’s really a problem, you can patch it:
[5:25]
https://github.com/helpshift/hydrox/blob/master/lein/src/leiningen/hydrox/setup.clj
GitHub
helpshift/hydrox
hydrox - Dive deeper into your code
dm3 [5:31 PM]
yeah, would have to patch cljs.tools.reader too unfortunately :confused:
[5:31]
think I'll just work around that
bronsa [5:36 PM]
@zcaudate: `::foo/bar` style keywords are in by design and not going anywhere, `:foo/bar/baz` have always been invalid by the spec and undefined behaviour (edited)
[5:37]
@dm3: is changing undefined behaviour a breaking change? :simple_smile:
dm3 [5:37 PM]
breaking as in breaking people's code :simple_smile:
[5:37]
e.g. zcaudate
bronsa [5:38 PM]
that code is already broken if it's using invalid clojure. It's just accidentaly working
dm3 [5:39 PM]
I'm looking from a pragmatic perspective. Theoretically you're right :simple_smile:
[5:40]
and I'm not judging either
bronsa [5:40 PM]
pragmatically, `:foo/bar/baz` is a bug waiting to happen. what does `(namespace :foo/bar/baz)` return?
dm3 [5:40 PM]
whatever it returns currently?
[5:41]
I mean it's kind of defined by the implementation
sveri [5:41 PM]
@dm3 @bronsa I wouldnt even agree that you are theoretically right. As soon as enough people adapted the broken code it falls under something like a common law that was accepted by both parties for a long enough time.
bronsa [5:42 PM]
what about `namespace` on `(keyword "foo/bar" "baz")` and `(keyword "foo" "bar/baz")`
dm3 [5:43 PM]
I agree that currently implemented semantics are messy
[5:43]
but my point was that it's still a breaking change
[5:43]
not that it's a "bad" change
[5:43]
that's a judgement
bronsa [5:43 PM]
@sveri: I would agree with you, as long as the undefined behaviour we accept as defined doesn't cause impossible to fix semantics.
[5:44]
that's why for example, the patch that made symbols starting with numbers illegal was rolled back
[5:45]
it broke existing code, it didn't cause weird semantics so it was rolled back. not the case with `:foo/bar/baz`
[5:45]
@dm3: you could make the point that fixing any bug is a breaking change then -- people might be relying on that bug.
dm3 [5:46 PM]
yep, I guess what matters is how obvious the incorrect behaviour is and how many people rely on it
bronsa [5:46 PM]
if the doc explicitely says "you can use *one* `/` inside a symbol", then if you're using more than one you're writing invalid clojure and you should expect it to maybe break (edited)
dm3 [5:47 PM]
I really haven't even thought about multiple slashes (nor noticed the docs) in a symbol in ~3 years of using Clojure
[5:47]
my initial thought (today) was that it was permitted
[5:47]
and the namespace would be the first segment before the first slash (edited)
bronsa [5:51 PM]
well, that doesn't make much sense though. `/` in clojure means `namespace separator`. if I see `FOO/BAR`, no matter what `FOO` and `BAR` are, I know that `FOO` is the namespace, and `BAR` is the name. if you want to express paths with keywords as in @zcaudate's lib, you should use a different separator in your keywords that doesn't have a special meaning in clojure, like `.` (i.e. `:foo/bar/baz` -> `:foo.bar.baz` or `:foo/bar.baz`) (edited)
dm3 [5:51 PM]
I do not want to argue semantics. Just sharing one point of view
bronsa [5:52 PM]
and my point is that pragmatic point of views (especially when they go against the current doc) should only be considered if the semantics they imply are clear and unambiguous
sveri [5:53 PM]
@bronsa: Nice explanation, thank you :simple_smile:
dm3 [5:53 PM]
yep :simple_smile:
[5:54]
I agree to that, as you have to make decisions in the end
zcaudate [9:07 PM]
@bronsa: written form of communication has a way of making things more serious than they seem
[9:08]
honestly… i knew it was coming since 1.6 when the edn reader started breaking my code
[9:09]
it’s probably more my fault for not communicating this earlier but oh well.. we all have to roll with the times
bronsa [9:09 PM]
@zcaudate: yeah no worries, I was just using your lib as an example since you brought that up
zcaudate [9:10 PM]
having said that, you can imagine my disappointment because I had designed an entire query semantic based on the keyword `:foo/bar/baz` feature (now bug) (edited)
[9:11]
http://docs.caudate.me/adi/adi-walkthrough.html#querying
[9:11]
you noticed I didn’t use `(adi/select ds {:student/classes/teacher/name "Mr. Blair"}})`
[9:12]
in my docs because the tests started breaking
jstew [9:12 PM]
@zcaudate: You put out so much quality stuff that I wonder if you ever sleep!
1
bronsa [9:12 PM]
@zcaudate: luckly the fix should be easy :simple_smile: just replace `/` with `.`
zcaudate [9:12 PM]
no!
[9:13]
see the problem is… datomic has things like `account.type/user`
[9:13]
and so I would have to do the cljs thing `account.type$user`
bronsa [9:13 PM]
(that's some high quality documentation btw, good job)
zcaudate [9:14 PM]
@bronsa: hahaha thanks… so maybe you can push the fix to 1.10
[9:14]
that way I can get a few more months left
[9:15]
like it’s not a big deal… but I thought that there is a parallel between the path structure of the `/` calls and the nesting of maps
[9:16]
and so there is an equivalence to `{:student {:classes {:teacher {:name '(?fulltext "Blair")}`
[9:17]
and `{:student/classes/teacher/name "Mr. Blair"}`
[9:17]
which is prettier in my opinion
bronsa [9:17 PM]
@zcaudate: sorry if that wasn't clear, but I don't actually have any control over when or what gets into clojure or not, I'm just a contributor :simple_smile: so there are chances that the clojure/core team will take a different decision and actually decline that ticket (I would be really disappointed if that was the case though!). If that will happenI will obviously make a change to `tools.reader` to allow them aswell, (edited)
zcaudate [9:18 PM]
@bronsa: damn.
[9:19]
well… maybe you can highlight this fact
[9:19]
and also if the fix is made, a fix to `(keyword "foo/bar/baz")` will also be needed (edited)
bronsa [9:20 PM]
don't think that'll ever be done. validating inputs to `keyword`/`symbol` etc has been asked/discussed tons of times and repeatedly declined for performance reasons
zcaudate [9:20 PM]
so that’s a matter of consistency.
bronsa [9:20 PM]
(not that I agree with that decision, but it seems like Rich isn't going ot change his mind on that)
[9:21]
@zcaudate: there's a difference between what a symbol/keyword can be at runtime, and what a valid read-time symbol/keyword is
zcaudate [9:21 PM]
and also, it means I can setup a reader macro #k foo/bar/baz and get the same effect
[9:22]
like it’s stupidly ugly but i believe it will work
bronsa [9:23 PM]
but the ambiguity of what to do with `namespace` and `name` still remains so dunno
[9:23]
@zcaudate: that wouldn't work either way, if
http://dev.clojure.org/jira/browse/CLJ-1530 gets accepted neither `:foo/bar/baz` *nor* `foo/bar/baz` will be valid anymore
[9:24]
@zcaudate: btw I'd suggest you log your issues with that ticket in a comment there if you feel strongly against it
new messages
[9:25]
I *suspect* that the response will be "you should use a delimiter that doesn't have a special meaning in clojure", but I might be completely wrong (I find the core team doesn't agree with my opinions quite frequently :) ) especially if you point out that your library will break. (edited)
zcaudate [9:30 PM]
@bronsa: thanks for the heads up. I’ll leave a comment and add a prayer for the bdfl