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

Making ns-qualified keywords not have font-lock-type-face #468

Open
vemv opened this issue Jan 20, 2018 · 7 comments
Open

Making ns-qualified keywords not have font-lock-type-face #468

vemv opened this issue Jan 20, 2018 · 7 comments

Comments

@vemv
Copy link
Contributor

@vemv vemv commented Jan 20, 2018

Given a ns-qualified keyword such as:

::kws.grid/column-settings

I'd like only clojure-keyword-face to be applied to it. But also font-lock-type-face is partially applied to it:

How it looks like

image

How it is parsed

#("::kws.grid/column-settings" 0 6 (fontified t face (clojure-keyword-face)) 6 10 (fontified t face (font-lock-type-face clojure-keyword-face)) 10 11 (fontified t face (default clojure-keyword-face)) 11 25 (fontified t face (clojure-keyword-face)) 25 26 (fontified t font-lock-multiline t face (clojure-keyword-face)))

The problematic bit seems to be (font-lock-type-face clojure-keyword-face).

I'm not sure if this is a feature. To me it looks buggy, because only grid gets the grey coloring. But in the example grid does not have a special meaning.


Using clojure-mode latest/pristine, no fork

@vemv
Copy link
Contributor Author

@vemv vemv commented Jan 20, 2018

In case it helps, this is the culprit:

      ;; foo/ Foo/ @Foo/ /FooBar
      (,(concat "\\(?:\\<:?\\|\\.\\)@?\\(" clojure--sym-regexp "\\)\\(/\\)")
       (1 font-lock-type-face) (2 'default))
@bbatsov
Copy link
Member

@bbatsov bbatsov commented Mar 28, 2018

@Bost I think your ns font-locking changes addressed this, right?

@Bost
Copy link
Contributor

@Bost Bost commented Mar 28, 2018

@Bost I think your ns font-locking changes addressed this, right?

Yes it does. (@vemv Bozo references to the 14d4221.)
A bit of an explanation - the ::kws.grid/column-settings is not a valid keyword. I assume we want to take a look at :kws.grid/column-settings, then:

user> ::kws.grid/column-settings
RuntimeException Invalid token: ::kws.grid/column-settings  clojure.lang.Util.runtimeException (Util.java:221)
user> :kws.grid/column-settings
:kws.grid/column-settings
user> (namespace :kws.grid/column-settings) ;; namespace font-locked as font-lock-type-face
kws.grid
user> (name :kws.grid/column-settings) ;; keyword names font-locked as clojure-keyword-face
column-settings
user>

And here is how is looks like:
image

So to answer:

(font-lock-type-face clojure-keyword-face) I'm not sure if this is a feature. To me it looks buggy. grid does not have a special meaning.

Yea you were looking at a buggy feature implementation, now hopefully fixed. Please get the 14d4221 and have a look again.

@vemv
Copy link
Contributor Author

@vemv vemv commented Mar 28, 2018

::kws.grid/column-settings is not a valid keyword

For the record: it is - if you :require x as kws.grid, then ::kws.grid/column-settings will become a valid keyword in the current ns/context.

Will checkout CIDER @ latest asap.

Thanks!

@Bost
Copy link
Contributor

@Bost Bost commented Mar 28, 2018

What??? Egh...

user> (require '[clojure.core :as kws.grid])
nil
user> ::kws.grid/fn
:clojure.core/fn
user> ::kws.grid/ufo
:clojure.core/ufo
user> (namespace ::kws.grid/ufo)   ;; I wonder how many folks get confused over this?
clojure.core
user> (name ::kws.grid/ufo)
ufo
user> (name :kws.grid/ufo)
ufo
user> (namespace :kws.grid/ufo)
kws.grid
user> 

egh... really. I think it would be a good idea to indicate it with some different font-face. @bbatsov?

@vemv
Copy link
Contributor Author

@vemv vemv commented Mar 28, 2018

(namespace ::kws.grid/ufo) might be bit of an unfair example ;p

One normally aliases using close terms (e.g clojure -> clj)

Thus, (namespace ::clj.cr/fn) returning :clojure.core/fn seems more defensible!

So a different font-fact may be overkill.

@MalloZup
Copy link

@MalloZup MalloZup commented Aug 5, 2019

autogenerated with https://github.com/MalloZup/doghub: issue inactive since 450 days. Please update the issue or close it

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.