Namespace font-locking according to clojure.lang.LispReader

[Bost]

Fix namespace alias font-locking for aliases containing non-letter
charactes like $, 0-9, _, etc.

[Malabarba]

Could you add a url link to the LispReader source here?

[expez]

If we're going to link I think we should link to a line in a specific revision, e.g. the tag for clojure 1.7. If just put a link to a line number in a file on master we might get in a situation where there's something else on that line or that the file itself has been moved/deleted.

[Bost]

Please see my improvements in cb62888.

[expez]

This links to a function invocation, at least to me this line:

IFn macroFn = getMacro(ch);

doesn't shed much light on anything discussed in this PR.

[Bost]

Well, basically I took a look at the for-loop https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LispReader.java#L232 consuming the input string in order to see which chars can be a part of a clojure symbol and which chars delimit string boundaries, lists, vectors, special forms etc. From this point of view the line 260 (together with line 254) is the key point of the for-loop.

[Bost]

@expez your note about linking to a specific clojure version (1.7) was right. I'm gonna fix that too.

[Bost]

I amended the cb62888 to fc5e092. Please review it.

[Malabarba]

Looks good to me.

[Malabarba]

Merged. Thanks again!

[bbatsov]

Just for the protocol - the second commit had a commit message, that doesn't match our standards and should have been simply squashed into the first one. Anyways, didn't have time to take a look at this until just now.

[bbatsov]

And a bit more feedback - I strongly prefer the usage of whitelists over blacklists as they tend to leave less probability for mistakes. The naming of the 3 constants is not exactly ideal either - one would hardly imagine those are blacklists.

[Malabarba]

Sorry. The thing with the commits escaped me indeed.
I merged with the intention of fixing some formatting anyway, so I'll make your changes too.

[bbatsov]

Sounds good.

[Malabarba]

I strongly prefer the usage of whitelists over blacklists as they tend to leave less probability for mistakes

Agreed, but in this case a whitelist is rather impractical. Clojure symbols are like elisp symbols in the sense that almost anything is a valid symbol char (e.g., (def ação😇✈ 10)). So it's easier to define the exclusions than the inclusions (even if we do miss a couple, as we did :).

[bbatsov]

Fair enough.

[Bost]

whitelist is rather impractical.

Yes I noticed that white vs. black list dissent too. Here I made a conscious decision "correctness before practicality". If authors of clojure.lang.LispReader decided for blacklist, so be it! I should follow their decision. An Emacs plugin is not the right place to question it.
Besides, if the attempts to write a whitelist regexps failed, then one should not repeat the same mistakes again and should write them the same way as they come over from the model (= LispReader).

The naming of the 3 constants is not exactly ideal either - one would hardly imagine those are blacklists.

???
I wrote "... blacklist... " to the docstrings! Moreover it's just an implementation detail if a regexp is a type-of blacklist or a type-of whitelist. A regexp matches or not a pattern in a string. How it is done should be abstracted away.
I see Artur already added "forbidden" to the names of the constants in questions. Uhm, I have a feeling a type-information leaks into the variable names. Don't you think we're complecting things here?
Except that:

1. I agree the variable names are not well chosen. I'm getting the impression I could better describe what they are good for.
2. Where can I find the commit message standards? (Sorry I missed them somehow.)

[Bost]

@Malabarba thanx for the "Add whitespace to clojure--sym-forbidden-rest-chars" commit. That was a mistake! I realized that only a day ago.

[bbatsov]

I'm quite fine with the naming changes Artur made. Variable names should convey meaning and the original names conveyed less meaning.

1. Where can I find the commit message standards? (Sorry I missed them somehow.)

CONTRIBUTING.md

[Malabarba]

Moreover it's just an implementation detail if a regexp is a type-of blacklist or a type-of whitelist. A regexp matches or not a pattern in a string. How it is done should be abstracted away.

Except the first two variables are not regexps (they lack the surrounding []). Technically they weren't blacklists either (they started with a ^ for use in a regexp).
I've changed the variables to be actual blacklists now, and that's why I changed the name to include the word forbidden.

Anyway, let's not bikeshed this too much. I believe we are all happy with the current state, right?

[Bost]

we are all happy with the current state

Yea. Sort of. Either way I started to work more on this regexp thing. I hope I'll sent you a PR withing next 2 or 3 weeks.

[Bost]

Technically they weren't blacklists either (they started with a ^ for use in a regexp).

That's a nit-picky remark... or(!) a fine demonstration of a sharp, bug-catching eye. I like that! I'll incorporate that in my PR. Thx Artur.