Bug 431011 - LSP Client.Local variables/members are not highlighted (all in one color)
Summary: LSP Client.Local variables/members are not highlighted (all in one color)
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: general (show other bugs)
Version: 20.12.0
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-31 16:53 UTC by Piotr Mierzwinski
Modified: 2021-04-29 23:48 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 21.04.0
Sentry Crash Report:


Attachments
kate-dark-mode-with-LSP (95.79 KB, image/png)
2021-03-15 22:59 UTC, Piotr Mierzwinski
Details
kate-21.04 semantic highlighting (110.33 KB, image/png)
2021-04-29 22:02 UTC, Piotr Mierzwinski
Details
kdevelop-semantic highlighting (97.52 KB, image/png)
2021-04-29 22:04 UTC, Piotr Mierzwinski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Mierzwinski 2020-12-31 16:53:25 UTC
SUMMARY
Semantic highlighting seems works for C/C++. Only minor issue is that local variables/members placed in body of methods, including parameters passed to them are not highlighted. 
I'm not sure if this depends on server working by default with clang.

STEPS TO REPRODUCE
1. Turn on LSP Client (default configuration)
2. Check 'Semantic highlighting'
3. Open any cpp file

OBSERVED RESULT
Local variables/members placed in body of methods, including parameters passed to them are not highlighted. 

EXPECTED RESULT
Local variables/members placed in body of methods, including parameters passed to them should be highlighted.

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: YES
(available in About System)
KDE Plasma Version: 5.20.4
KDE Frameworks Version: 5.77.0
Qt Version: 5.15.2

ADDITIONAL INFORMATION
Comment 1 Bug Janitor Service 2021-01-22 13:25:32 UTC
A possibly relevant merge request was started @ https://invent.kde.org/utilities/kate/-/merge_requests/195
Comment 2 Christoph Cullmann 2021-01-23 08:54:06 UTC
Git commit 010b6c711112d82e804a844b5e3a07839ea9c09e by Christoph Cullmann, on behalf of Waqar Ahmed.
Committed on 23/01/2021 at 08:53.
Pushed by cullmann into branch 'master'.

Use the new theme function to provide better semantic highlighting

This change is a second proposal for better semantic highlighting for
C++. It can be considered as temporary till we are able to support for
new API and till LSPs themselves add proper support for offical semantic
tokens.

Since we rely on the new API call (theme()), this change will only work
for KF > 5.79.
Related: bug 431014

M  +1    -0    addons/lspclient/CMakeLists.txt
M  +6    -2    addons/lspclient/lspclientpluginview.cpp
M  +7    -0    addons/lspclient/lspclientprotocol.h
M  +6    -1    addons/lspclient/lspclientserver.cpp
A  +195  -0    addons/lspclient/lspsemantichighlighting.cpp     [License: MIT]
A  +76   -0    addons/lspclient/lspsemantichighlighting.h     [License: MIT]

https://invent.kde.org/utilities/kate/commit/010b6c711112d82e804a844b5e3a07839ea9c09e
Comment 3 Piotr Mierzwinski 2021-03-15 22:59:34 UTC
Created attachment 136715 [details]
kate-dark-mode-with-LSP
Comment 4 Piotr Mierzwinski 2021-03-15 23:06:07 UTC
Seems this fix doesn't work. Yesterday I got update with KF-5.80. Today tested Kate. Result please find in attache screenshot. Local variables still are not coloured differently - all are white.

Color Scheme is "Breeze-Dark"
Color Theme is  "Breeze-Dark"
Comment 5 Waqar Ahmed 2021-03-16 10:47:35 UTC
This feature is added in Kate not frameworks. So frameworks update doesn't mean this will be fixed. 

Please wait for "Kate 21.04" release to come out or build from source.
Comment 6 Piotr Mierzwinski 2021-04-29 22:01:40 UTC
Since some time I have Kate 21.04, so I decided to test this fix.
In my humble opinion this fix (considered as temporary)  doesn't work at all.  Local variables still have the same colour! Only one change I noticed is that types changed colour to much brightest (for dark theme ), so they are more readable. Only this wasn't reported here as issue.
Please check attached screenshot and compare to one took from KDevelop.

I assume that we need to wait "we are able to support for new API and till LSPs themselves add proper support for offical semantic tokens."

SOFTWARE/OS VERSIONS
KDE Plasma Version: 5.21.4
KDE Frameworks Version: 5.81.0
Qt Version: 5.15.2 (with KDE patches)
KApplications 21.04
Comment 7 Piotr Mierzwinski 2021-04-29 22:02:26 UTC
Created attachment 138016 [details]
kate-21.04 semantic highlighting
Comment 8 Piotr Mierzwinski 2021-04-29 22:04:17 UTC
Created attachment 138017 [details]
kdevelop-semantic highlighting
Comment 9 Waqar Ahmed 2021-04-29 22:22:45 UTC
1. We don't color all the information provided by clangd at the moment, some is ignored. This was intentionally done by me to not color everything there is in the source file. What gets highlighted;
- functions
- types
- parameters
The list is incomplete since I don't remember all the details. 

2. Even with semantic token API things will likely not change. 
3. Kdevelop has an entirely different way of doing this, they use libclang directly.
Comment 10 Waqar Ahmed 2021-04-29 22:58:00 UTC
Another reason and I think this is the main reason for not doing more coloring is because we don't have that big of a color palette. A usual color scheme has about 5 - 7 colors, of which one is normal text. We simply can't change this because it's not in our hands, so we have to work with this. Introducing hard Coded colors like kdevelop is not an option.
Comment 11 Piotr Mierzwinski 2021-04-29 23:19:04 UTC
> 1. We don't color all the information provided by clangd at the moment, some is ignored. 
> This was intentionally done by me to not color everything there is in the source file. What gets 
> ....
In my opinion semantic highlighting in KDevelop significantly increase readability of code.  I consider  this is the pattern of how it should work.  Readability of code, thanks to usage of semantic highlighting is very important, at least for me.

Why you don't color all the information provided by clangd ? Is this technical reason or any different?
Why this cannot be configurable like in KDevelop where we can increase/decrease its intensity?

And I don't understand if you don't want to achieve or even pursue level of semantic highlighting provided by KDevelop or this is impossible because of some technical reasons.


> 2. Even with semantic token API things will likely not change. 
So seems I misunderstood what Christoph said about this API.  Notice please, he considered this solution like temporary. And to be honest, after usage mentioned API and semantic tokens, I expected better highlighting in Kate, which will be comparable to one is present in KDevelop . I know it uses different way do it.
Anyway tell me please what will bring new API and support for semantic tokens?

And last thing, which was reported here as a bug. 
Should I understand that local variables will be not colored in future version of Kate, I mean in such way that each of them has  different color, like in KDevelop?
Comment 12 Piotr Mierzwinski 2021-04-29 23:29:31 UTC
(In reply to Waqar Ahmed from comment #10)
> Another reason and I think this is the main reason for not doing more
> coloring is because we don't have that big of a color palette. A usual color
> scheme has about 5 - 7 colors, of which one is normal text. We simply can't
> change this because it's not in our hands, so we have to work with this.

Sorry? You don't have such bgi of color palette? So how semantic highlighting implemented in KDevelop manges color palette? I think it uses shades of colors to increase palette. Isn't possible to do it in Kate?
You have 5-7 colors, but I think is possible use shades of color for each of them.
I don't understand what is not in your hands? That there is available only 5-7 colors. Not all need to be in completely different color.

> Introducing hard Coded colors like kdevelop is not an option.
And is is solution also. We do not have to be condemned to 7 colors.
Comment 13 Waqar Ahmed 2021-04-29 23:48:16 UTC
Kdevelop has some hardcoded color palette that it uses for semantic highlighting. That makes it not configurable at all. This is enough of a reason that this is not an option at all.

We are limited to the themes color palette, because we want to stick to the theme. Introducing shades means breaking that, hence not an option. This is one reason color schemes are useless in Kdevelop because semantic highlighting basically removes all the originality of the theme. 

New API will not change anything from a user's perspective. It is just the official API that we have to support so that we can highlight languages other than C/C++. It may have some performance benefits as well.

And yes, differently colored local variables aren't possible in Kate and will likely never happen, simply because no language server provides support for it and I don't think they ever will.