r-lib / downlit

Syntax Highlighting and Automatic Linking
https://downlit.r-lib.org
Other
90 stars 22 forks source link

Same chroma class:`NULL` and `if` as well as `library()` and other function names. #31

Closed TimTeaFan closed 4 years ago

TimTeaFan commented 4 years ago

I am using {hugodown} and the academic theme to create a website. I try to recreate my favorite RStudio highlighting theme "cobalt". This is were I encountered the following issues:

(1) It seems like {downlit} assigns the same class kr(KeywordReserved) to NULL and if:

Bildschirmfoto 2020-07-22 um 22 41 24

highlight.js's R pattern uses here two different classes (similar to RStudio) called hljis-keyword and hljs-literal :

Bildschirmfoto 2020-07-22 um 22 40 45

Although I am still fiddling around (a lot), I think this has nothing to do with my setup, but is rather an issue of how the chroma classes are defined in downlit::highlight() here and here.

My question is: Is there a reason NULL and if are assigned to the same class, and could this be fixed?

Add on: (2) A related issue is that library() calls are assigned the same class as regular functions (e.g. print()), both are of class nf (NameFunction). It would be great to have a separate class for library() calls, since RStudio highlights them differently. Here again highlight.js is making a distinction.

hadley commented 4 years ago

Sure, if you can suggest what class you think would be more appropriate.

TimTeaFan commented 4 years ago

I’d say that if is correctly identified as "KeywordReserved" (class "kr"). For NULL there seems to be no obvious choice. I’d go with "literal" (class "l"). This is consistent with highlights.js’s terminology, and this class seems not to be in use yet by downlit::highlight().

Regarding functions that have (depending on the theme) special highlighting rules in RStudio, such as library(), function(), UseMethod() etc., I’d suggest using "NameFunctionMagic" (class "fm"). Although not 100% equivalent to pygments defintion I’d say it comes close, and this class seems not to be in use yet too.

hadley commented 4 years ago

Yeah, literal looks like a good choice to me for NULL.

I'm not sure if I want different highlighting for special functions because I think the RStudio list is rather idiosyncratic.

TimTeaFan commented 4 years ago

That’s a really valid point. The RStudio highlighting rules are somewhat arbitrary. When looking at the three examples from a functionality perspective,library() seems just like a normal function call, whereas function() and UseMethod() are rather special (defining) meta functions. From a readability perspective, however, I do see why library() and function() get a special (highlighting) treat: it helps to focus on the function arguments. Here, highlighting UseMethod() doesn’t make much sense, since the function argument is a string, which is already highlighted differently.

Personally, I do prefer the readability argument and would welcome library() and function() to be highlighted differently.

hadley commented 4 years ago

I'm going to leave off syntax highlighting special functions for now, but we can revisit in the future if others find it important.