Skip to content

New highlighter based on tokens and untyped trees #5137

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

Merged
merged 22 commits into from
Sep 26, 2018

Conversation

allanrenucci
Copy link
Contributor

No description provided.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@allanrenucci
Copy link
Contributor Author

CLA is signed by all contributors:

The dotty bot is confused once again. @smarter this is ready for review

Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

case tree: MemberDef /* ValOrDefDef | ModuleDef | TypeDef */ =>
for (annotation <- tree.rawMods.annotations)
highlightPosition(annotation.pos, AnnotationColor)
val color = if (tree.isInstanceOf[ValOrDefDef]) ValDefColor else TypeColor
Copy link
Contributor

Choose a reason for hiding this comment

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

Use pattern match instead of isInstanceOf

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe could just be another case in the outer pattern match and for (annotation <- ... could go in a helper method.

@@ -284,7 +284,9 @@ class ReplDriver(settings: Array[String],
x.symbol
}
.foreach { sym =>
out.println(SyntaxHighlighting("// defined " + sym.showUser))
// FIXME syntax highlighting on comment is currently not working
Copy link
Contributor

Choose a reason for hiding this comment

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

Will need to open an issue for this

slothspot and others added 14 commits September 26, 2018 21:44
- Highlight is based on MemberDef with annotations present
- move scanner highlighter before parser so that annotation @inline
wholdn't be highlighted as keyword
- annotations are highlighted when full member definition is available
If we ignore trees named <error> and <init>, it is not so
fragile after all...
Prior to this commit, an annotation with one or more parameter lists
would have incorrect positions attached to each parameter list.  For
example, the annotation

    @foo("bar")

would have the erroneous position `<4..10>` attached to its `Apply` node
instead of the correct position `<4..11>`.  This was caused by forget-
ting to take into account the closing parenthesis.
These tests currently fail because of a bug in the syntax highlighter.
danielyli and others added 8 commits September 26, 2018 21:44
Test for `1Lx` currently fails; tests for float literals pass.
The end position was correctly set but the start position was wrong: the
start position of `fn(args)` should be the same as the start position of
`fn` itself.
Tokens position is less fragile. Specially when code does not parse
@allanrenucci allanrenucci merged commit 7336594 into scala:master Sep 26, 2018
@allanrenucci allanrenucci deleted the new_highlighter branch September 26, 2018 20:18
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

Successfully merging this pull request may close these issues.

6 participants