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

else|break|next|repeat|in as Name.Function instead of Name #2087

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vladbazon
Copy link

No description provided.

@jeanas jeanas linked an issue Mar 13, 2022 that may be closed by this pull request
Copy link
Contributor

@jeanas jeanas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this changes the output, you have to regenerate the tests for these languages with, e.g., pytest --update-goldens tests/examplefiles/splus.

@@ -89,7 +89,7 @@ class SLexer(RegexLexer):
(r'\[{1,2}|\]{1,2}|\(|\)|;|,', Punctuation),
],
'keywords': [
(r'(if|else|for|while|repeat|in|next|break|return|switch|function)'
(r'(if|for|while|return|switch|function)'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused. This rule is apparently broken, since include('valid_name') comes before include('keywords') in the 'statements' state. Do you really want to highlight these as functions and not as keywords? How about simply moving include('valid_name') after include('keywords')? Could you explain the reason why else, repeat etc. are different from if, for, etc.?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been looking at the R specification and it does claim that things like "break" are indeed functions in https://cran.r-project.org/doc/manuals/r-release/R-lang.html#Flow-control-elements, but, in other sections they're considered keywords:

I'm inclined to not change them to Function because they seem quite different from actual functions like mean etc.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to not change them to Function because they seem quite different from actual functions like mean etc.
library(highr) ## in the R console
highr:::cmd_latex
cmd1 cmd2
COMMENT \hlcom{ }
FUNCTION \hlkwa{ }
IF \hlkwa{ }
ELSE \hlkwa{ }
WHILE \hlkwa{ }
FOR \hlkwa{ }
IN \hlkwa{ }
BREAK \hlkwa{ }
REPEAT \hlkwa{ }
NEXT \hlkwa{ }
NULL_CONST \hlkwa{ }
LEFT_ASSIGN \hlkwb{ }

The same highlighting LaTeX command
\hlkwa{ ... }
is used for FUNCTION as for ELSE, BREAK, NEXT, IN, REPEAT

But in SLexer ("r.py") actually highlight only function names (if followed by '(' evidently), not the keyword 'break', 'else' etc.
(except when they are followed by '(' - which is correct, but not usual; we write "for i in 1:5", not "for i in() 1:5")

--vladbazon

@vladbazon
Copy link
Author

vladbazon commented Mar 13, 2022 via email

@jeanas
Copy link
Contributor

jeanas commented Mar 14, 2022

@Leonel020681 That's our CI workflow definition. Not sure what you mean by copying it in your comment?

@pygments pygments deleted a comment from Leonel020681 Mar 14, 2022
@Anteru
Copy link
Collaborator

Anteru commented Mar 14, 2022

@jean-abou-samra Probably a bot/spam

@jeanas jeanas added the update needed Waiting for an update from the PR/issue creator label Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update needed Waiting for an update from the PR/issue creator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SLexer - break, next, else, in, repeat as Name.Function, not Name
4 participants