Make the ns regexp more permissive #350
Conversation
| @@ -1009,7 +1009,7 @@ nil." | |||
| (one-or-more (any whitespace "\n"))) | |||
| ;; why is this here? oh (in-ns 'foo) or (ns+ :user) | |||
| (zero-or-one (any ":'")) | |||
| (group (one-or-more (not (any "()\"" whitespace))) word-end))) | |||
| (group (one-or-more (not (any "()\"" whitespace))) symbol-end))) | |||
There was a problem hiding this comment.
Not sure how correct our symbol definition is. If memory serves - not very correct. This change definitely requires unit tests illustrating it actually solves something.
There was a problem hiding this comment.
Our symbol definition is inherited from elisp so it's at least better than using the word definition. I can't think of any notable differences in clojure's and elisp's notion of what a symbol is.
There was a problem hiding this comment.
The problem is that I tried using symbol instead of word semantics when I inherited the mode and everything broke, therefore my reservations... Using symbol semantics is the reasonable thing to do for pretty much everything, but I really think something was fucked up regarding them (don't remember what). I recall lisp-mode's code was full of word references itself, which was odd.
At any rate - a few unit tests won't hurt.
There was a problem hiding this comment.
AFAICT, that regexp should be just (1+ (any symbol word)). But you're the voice of experience here @bbatsov.
There was a problem hiding this comment.
That sounds reasonable. But on a general note @expez is right that there should be fewer word checks and symbols checks, as we're talking about symbols here.
There was a problem hiding this comment.
Btw, most of this word stuff predates me becoming the maintainer of clojure-mode. I always wanted to redo it (and unify the regex syntax used in clojure-mode), but never had the time to do.
There was a problem hiding this comment.
AFAICT, that regexp should be just (1+ (any symbol word)).
Correction, what I wrote is not valid rx. It would have to be (1+ (or (syntax word) (syntax symbol)))
expez
force-pushed
the
update-ns-regexp
branch
from
ba333d7 to
b61fbc6
Compare
|
expez
force-pushed
the
update-ns-regexp
branch
from
b61fbc6 to
f344852
Compare
|
Actually, I've restored the regexp to my first suggestion, because (group (one-or-more (or (syntax symbol) (syntax word))))fails on namespaces like this: |
|
Ideally, each of those tests would also check that |
|
Apart from @Malabarba's suggestion - a changelog entry would be nice. No other remarks from me. P.S. We should turn more comments into specs. :-) |
expez
force-pushed
the
update-ns-regexp
branch
6 times, most recently
from
7a277dd to
01568a3
Compare
Any valid symbol can be used to name an ns. The current regexp would fail to parse symbols ending in certain characters, e.g. `+`. This closes clojure-emacs/cider#1433
expez
force-pushed
the
update-ns-regexp
branch
from
01568a3 to
9b7885e
Compare
|
Aright let's go |
Make the ns regexp more permissive
|
👍 |
Any valid symbol can be used to name an ns. The current regexp would fail
to parse symbols ending in certain characters, e.g.
+.This closes clojure-emacs/cider#1433