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

Fix Highlighter #716

Merged
merged 5 commits into from
Nov 22, 2022
Merged

Fix Highlighter #716

merged 5 commits into from
Nov 22, 2022

Conversation

HoshinoTented
Copy link
Contributor

There are some problems with Highlighter

  • Expr.Param#refs didn't highlight (so no definitions)
  • For the parameters that share the same type, like (a b : A), the type will be highlighted twice, we can perform .distinct after .sort to solve this problem

@HoshinoTented
Copy link
Contributor Author

But kala seems no .distinct in SeqView:

  • use Java Stream
  • build a Set then toImmutableSeq()

@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Merging #716 (53eb75e) into main (732e6f3) will increase coverage by 0.00%.
The diff coverage is 81.08%.

❗ Current head 53eb75e differs from pull request most recent head ee17be5. Consider uploading reports for the commit ee17be5 to get more accurate results

@@            Coverage Diff            @@
##               main     #716   +/-   ##
=========================================
  Coverage     81.51%   81.51%           
- Complexity     3009     3013    +4     
=========================================
  Files           262      262           
  Lines          9569     9577    +8     
  Branches       1192     1193    +1     
=========================================
+ Hits           7800     7807    +7     
  Misses         1119     1119           
- Partials        650      651    +1     
Impacted Files Coverage Δ
.../java/org/aya/resolve/module/FileModuleLoader.java 0.00% <0.00%> (ø)
...i/src/main/java/org/aya/cli/repl/ReplCompiler.java 75.34% <66.66%> (ø)
...ain/java/org/aya/cli/literate/SyntaxHighlight.java 62.12% <85.71%> (+2.12%) ⬆️
.../src/main/java/org/aya/core/serde/CompiledAya.java 84.81% <100.00%> (ø)
base/src/main/java/org/aya/generic/Constants.java 84.61% <100.00%> (ø)
base/src/main/java/org/aya/ref/LocalVar.java 100.00% <100.00%> (ø)
...ase/src/main/java/org/aya/resolve/ResolveInfo.java 100.00% <100.00%> (ø)
...main/java/org/aya/resolve/module/ModuleLoader.java 93.75% <100.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@imkiva
Copy link
Member

imkiva commented Nov 22, 2022

I kinda think we should not .sorted().distinct() every time we call highlighter, instead, let callers sort and distinct them. Because there are places we don't care about the order and duplication (like LSP).

@HoshinoTented
Copy link
Contributor Author

I kinda think we should not .sorted().distinct() every time we call highlighter, instead, let callers sort and distinct them. Because there are places we don't care about the order and duplication (like LSP).

I agree.

@mio-19
Copy link
Contributor

mio-19 commented Nov 22, 2022

lgtm
p.s. 不小心用同学的gh号评论了

@HoshinoTented HoshinoTented marked this pull request as ready for review November 22, 2022 12:08
@ice1000 ice1000 added this to the v0.24 milestone Nov 22, 2022
@ice1000
Copy link
Member

ice1000 commented Nov 22, 2022

bors r+
KISS

@bors
Copy link
Contributor

bors bot commented Nov 22, 2022

Build succeeded:

@bors bors bot merged commit 506d13c into main Nov 22, 2022
@bors bors bot deleted the fix-highlight-param branch November 22, 2022 21:08
@ice1000 ice1000 added the highlighter Faithful one label Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants