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

Theia is producing unexpected semantic highlightings for c++. #5854

Closed
jvikstrom opened this issue Aug 5, 2019 · 6 comments
Closed

Theia is producing unexpected semantic highlightings for c++. #5854

jvikstrom opened this issue Aug 5, 2019 · 6 comments
Labels
cpp issues related to the C/C++ language

Comments

@jvikstrom
Copy link

Description

Theia's semantic highlighting client is producing unexpected colorings in certain cases after enabling it for the c++ client. (look at this PR here for a reference on how to do it.

I've verified that clangd sends the correct source ranges for the test cases below.

Reproduction Steps

  • Enable semantic highlighting in the c++ client.
namespace aaa {
    void foo();
}
void foo() {
    aaa::foo();
}

In this case theia will color the aaa::foo(); incorrectly. The entire aaa::foo range will be highlighted as a function. Clangd sends the aaa range as a namespace and the foo range as a function.
On thing I tested was to add a custom rule to the color theme for namespaces (I gave namespaces a gray color). When doing that the aaa part will be highlighted as a namespace (which is correct). But the ::foo will be highlighted as a function. So in this case it almost seems to work correctly, but the :: should not be highlighted as a function.

Another case (probably related to the one above though) is this one.

namespace aaa {
  struct A {
    static void f();
  };
}
class A {};
void foo() {
  aaa::A::f();
  aaa::A a;
}

Here without the custom namespace rule the entire aaa::A::f range is highlighted as a function. When adding the custom namespace rule aaa is highlighted as a namespace, but the ::A::f range is highlighted as a function.
However, the aaa::A a line is highlighted correctly. The :: is not highlighted here. aaa is a namespace, A is a class and a is a variable.

But this is as far as I've looked into it, I don't know the theia code base but I hope this can help someone track this bug down.

OS and Theia version:
This happens on the latest master after enabling semantic highlighting the same way done in this PR.

@akosyakov akosyakov added the cpp issues related to the C/C++ language label Aug 5, 2019
@HighCommander4
Copy link
Contributor

I checked and the arguments being passed by the semantic highlighting service to TextEditor.deltaDecorations() here look correct to me. I think the bug is somewhere deeper.

It's possible we're running into this bug / limitation in the monaco-editor component.

@svenefftinge svenefftinge changed the title Thei is producing unexpected semantic highlightings for c++. Theia is producing unexpected semantic highlightings for c++. Aug 6, 2019
@HighCommander4
Copy link
Contributor

In #5941, I have implemented a proposed workaround for microsoft/monaco-editor#1070 in Theia. I'm not sure if it's the right approach, and even if it is the patch isn't complete, but it does fix this issue as well, confirming that it's the same underlying cause.

@kittaakos
Copy link
Contributor

Closed via #5941.

@Menci
Copy link

Menci commented Mar 6, 2020

I'm also working on adding custom semantic highlight on Monaco Editor. How did Theia add colors to the minimap?

@kittaakos
Copy link
Contributor

add colors to the minimap?

I think it's not supported.

From here:

decorations are not being drawn to the minimap, which means that the tokens that come from the semantic highlighting won't be shown in the minimap

@Menci
Copy link

Menci commented Mar 6, 2020

Yes, I see Theia is using deltaDecorations too. But Theia have minimap, isn't it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp issues related to the C/C++ language
Projects
None yet
Development

No branches or pull requests

5 participants