hylang / hy-mode

Hy mode for Emacs
GNU General Public License v3.0
189 stars 48 forks source link

Questions/suggestions about some font face decisions #31

Closed mhavard999 closed 6 years ago

mhavard999 commented 9 years ago

First off, great mode! Second, I have a list of casual suggestions/points that aren't demands that you can totally ignore. I don't want to sound demanding. I mainly just want to get your thoughts on these potential changes and then possibly make a pull request with any changes you agree with. All of these changes are related to what I've seen in other lisp modes and python-mode.el (not the built-in python mode since I don't use that so I don't know about it's font-face decisions).

I think that's all I have. I hope I don't come across as "Your mode sucks! Python and Clojure are better!" because I really don't intend to. This mode is great. I just didn't want to make a pull request with a bunch of dumb changes you'd never accept. Thanks!

theanalyst commented 9 years ago

On Thu, Apr 16, 2015 at 7:54 AM, mhavard999 notifications@github.com wrote:

First off, great mode! Second, I have a list of casual suggestions/points that aren't demands that you can totally ignore. I don't want to sound demanding. I mainly just want to get your thoughts on these potential changes and then possibly make a pull request with any changes you agree with. All of these changes are related to what I've seen in other lisp modes and python-mode.el (not the built-in python mode since I don't use that so I don't know about it's font-face decisions).

Actually this first one I believe is a bug - import is highlighted correctly only when you do (import sys) but if you do (import [sys [*]]) it does not highlight. I do agree with the choice of keyword-face :-)

Agree, this is a bug.. fixes accepted :-)

(require is fine since (require sys) is the only way to use it)

name in python-mode.el uses font-lock-constant-face, but hy-mode currently does not highlight it at all. I guess hy-mode doesn't highlight any of the special methods, this too needs to be added

(def var "foo") in clojure-mode or (defvar var "foo") in other lisp modes highlights def/defvar with font-lock-keyword-face and the variable name with font-lock-variable-name-face. python-mode.el uses the variable name face for variable names in assignments like var = "foo". Shouldn't def and setv follow this behavior? In python-mode.el, self is a keyword. It currently has no highlighting. Since hy has classes, it makes sense to highlight self

Sure both of these can be fixed easily enough I guess...

In clojure-mode, functions beginning with a dot (method names) are highlighted as keywords. I could honestly go either way on this. This one is probably too personal too include, but I'll mention it anyway. In hy, decorators are not referred to using the @ operator, but you can write a short reader macro - (defreader @ [fname] `~fname) - so that #@.* refers to a function. In python-mode.el, @decorator's are highlighted with the type-face.

Not completely sure on this one, since it may not be a reader macro many may use, but I guess we could probably highlight

differently?

In both clojure-mode and python-mode.el, and and or are highlighted as keywords, but currently they are highlighted with font-lock-built-in-face. int and float are highlighted built-ins in python-mode.el. Funnily enough, float? is highlighted this way. filter is highlighted as a built-in, but I really think map and reduce should be too, not only because they are in python-mode.el, but also because filter, map, and reduce are so important to functional programming. not is highlighted as a built-in, but in python-mode.el it is highlighted as a keyword I guess when hy-mode was written these weren't a part of hy as well, you could compare the bits in exports in language.hy and add the missing ones with the respective faces

I think that's all I have. I hope I don't come across as "Your mode sucks! Python and Clojure are better!" because I really don't intend to. This mode is great. I just didn't want to make a pull request with a bunch of dumb changes you'd never accept. Thanks!

Thanks for the detailed feedback :)

— Reply to this email directly or view it on GitHub.

ekaschalk commented 6 years ago

Thanks for the feedback. I've resolved several of these issues (imports, self) and will tackle the rest soon.

ekaschalk commented 6 years ago

Addressed all points.