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

Why highlight types and constants? #20

Open
isker opened this issue Jun 2, 2022 · 4 comments
Open

Why highlight types and constants? #20

isker opened this issue Jun 2, 2022 · 4 comments

Comments

@isker
Copy link

isker commented Jun 2, 2022

Hi. Thanks for this nice theme.

After trying it out for a while, I don't understand why so much is being highlighted in yellow.

Here's an example with a random illustrative file from the deno repository:
Screen Shot 2022-06-02 at 16 52 49

false, true, null, number literals, TextEncoder, Promise, etc are all highlighted here, due to usages of stimmung-themes-light-highlight-color here:

;; syntax
`(font-lock-builtin-face ((t (:background ,stimmung-themes-light-highlight-color :italic t))))
`(font-lock-comment-delimiter-face ((t (:background ,str :italic t))))
`(font-lock-comment-face ((t (:background ,str :italic t))))
`(font-lock-doc-face ((t (:background ,str :italic t))))
`(font-lock-constant-face ((t (:background ,stimmung-themes-light-highlight-color :italic t))))
`(font-lock-function-name-face ((t (:foreground ,fg1 :bold t))))
`(font-lock-keyword-face ((t (:foreground ,fg1))))
`(font-lock-type-face ((t (:background ,stimmung-themes-light-highlight-color))))
`(font-lock-variable-name-face ((t (:foreground ,fg1 :bold t))))
`(font-lock-negation-char-face ((t (:foreground ,fg1))))
`(font-lock-preprocessor-face ((t (:foreground ,fg1))))
`(font-lock-preprocessor-char-face ((t (:foreground ,fg1))))
`(font-lock-regexp-grouping-backslash ((t (:foreground ,fg1 :bold t))))
`(font-lock-regexp-grouping-construct ((t (:foreground ,fg1 :bold t))))
`(font-lock-string-face ((t (:foreground ,fg1 :background ,str))))
`(font-lock-warning-face ((t (:foreground ,fg1 :underline (:style wave :color ,warning)))))
`(font-lock-doc-markup-face ((t (:background ,stimmung-themes-light-highlight-color))))

It seems a bit much, and it's not clear why constants and typenames deserve to be highlighted. I could of course override this, but I'd also like to hear what you think.

@motform
Copy link
Owner

motform commented Jun 6, 2022

Hi @isker,

Glad the theme peaked your interest! I think the discussion of what to highlight is very interesting, and in the end, it probably comes down to personal preference.

The selection of what to highlight is primarily influenced from two things: alabaster and writing Clojure code. In alabaster, Tonsky highlights constants and global definitions along with comments. I agree with this since I think that there is a conceptual and practical difference between programmatic behaviour and values "baked" into the code itself, i.e. constants. The latter are solely dependant on the source file as such, and therefore I think it makes sense to highlight them.

SCR-20220606-gqr

Of course, this can sometimes lead to large parts of yellow in code, as above, but that is also the point. The hiccup/html of Text is hard coded and clearly conceptually different to the other functions, whose behavior is input dependent to a higher degree.

However, I agree with you that the decision to highlight types is less clear cut. The initial reason for this is that I mostly program in Clojure, which is dynamically typed. As such, the presence of a type hints marks a rarity that I think make sense to highlight:

example here
Look for ^java.io.BufferedReader

In addition, font-lock-type-face is used to denote namespaces in when referring to something, ie **rf**/dispatch (as seen above). This also makes sense to me to highlight as we are "getting" something from another place, a clear boundary is passed in the code. To me, the same arguments sort of makes sense when dealing with types that are something other than primates, ie objects from another namespace. In your example, we are crossing a boundary that is significant by introducing a Promise, something that has a high level of impact to the rest of code (we now have to deal with it using a specific programming model).

That would be my "conceptual" argument anyway. On a visceral level however, I do kind of agree with you. The times when I program in something like JS I do sometimes feel like highlighting can be a bit "off", although I'm more bothered by how bolded method calls in changing blotch the page.

Does that make sense to you? I don't think I'll change the default as I'm against breaking changes once something hits a non-trivial amount of users, but one could image a way to programatically set which parts you would like to have highlighted with something like (setq stimmung-themes-highlight '('types 'variable-name)). The yellow is a bit decisive, which I why I also added the customisable variable stimmung-themes-[light|dark]-highlight-color.

@isker
Copy link
Author

isker commented Jun 10, 2022

Thanks for your detailed explanation. The screenshots remind me that I really have to patch my font height 🌞.

If I stare long enough at Text, it maybe starts to make sense. I think what I'm having trouble getting over is that primitives/constants seem like a strange thing to highlight so vibrantly, of all the things you could. But maybe I can train myself to think that the yellow does not necessarily mean "look at me, I'm important"...

I should have elaborated earlier that it seems strange that strings have a fixed gray background and other primitives/constants get highlighted. I might actually try making the highlight-color the same gray95 that strings get.

You're right that it's all down to personal preference. And I agree that changing defaults is not a good idea. It looks much better in clojure, for sure. I'm not sure that something like stimmung-themes-highlight is necessary, especially if the defaults are not going to be changing. Might as well tweak the theme directly.

With respect to highlighting types, I've been meaning to look into tree sitter and emacs, and this provides a motivator: in languages with lots of types in code, a good compromise might be to highlight only types of function parameters or return values, for example. Of course I have no idea if tree-sitter's emacs integration even supports that kind of thing.

@slotThe
Copy link
Contributor

slotThe commented Jun 24, 2022

I've also thought about this a little. In particular, I think comments can be a problem as well sometimes in terms of how "heavy" they feel. Consider the following example, especially in contrast with the comments just having some shade of grey as their fg colour:

2022-06-24-081657_769x726_scrot 2022-06-24-081718_758x706_scrot

I think it would be nice to be able to customise this to a certain extend. One can of course already overwrite certain faces by using custom-theme-set-faces but something "more official" might be neat as well.

motform added a commit that referenced this issue Jul 5, 2022
@motform
Copy link
Owner

motform commented Jul 5, 2022

Sorry for getting back so late! I think you both make very good points, it makes sense to afford the user the ability to change what they deem worthy of highlighting. I added user facing variables for each font-lock face, explained per the readme. Please give me some feedback if you think this approach makes sense and if there is something you feel is missing!

One, arbitrary, limitation right now is that you are limited to setting 'background, 'foreground or 'none. Font variant and the color of the highlight is still selected by the theme. This is mostly to prevent a combinatorial enum explosion, but one could also see a version that accepts either a simple version like (setq stimmung-themes-comment 'foreground) or a complex one `(setq stimmung-themes-comment '('foreground :bold t :highlight "goldenrod")), but that that point, one might as well override the face directly (although an opinionated interface might be useful?).

As a reference, Alabaster provides separate fore- and background highlight themes, which there is clearly som merit to, but I think a fully customisable approach makes more in the context of Emacs.

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

No branches or pull requests

3 participants