chylex / IntelliJ-Inspection-Lens

IntelliJ platform plugin that shows errors, warnings, and other inspection highlights inline.
https://plugins.jetbrains.com/plugin/19678-inspection-lens
Mozilla Public License 2.0
57 stars 12 forks source link

Adding background using RangeHighlighter #14

Closed synopss closed 1 year ago

synopss commented 1 year ago

Background can be added with RangeLineHighlighter.

I made some tests to add a background color while still having errors written in Inlays. It was working fine but the implementation was a bit hacky. I'll try to find a better solution before going for a PR.

Edit : after some research, I found EditorLinePainter. The idea is to replace Inlays by that while using RangeLineHighlighter for the background.

chylex commented 1 year ago

I cannot find any references to RangeLineHighlighter in the SDK.

synopss commented 1 year ago

I made a typo mistake, it's called RangeHighlighter You can use MarkupModel.addLineHighlighter()

chylex commented 1 year ago

I'm not familiar with these APIs, are there any caveats with replacing inlays with EditorLinePainter?

synopss commented 1 year ago

I can't tell you for sure, I am a bit like you digging into intellij APIs trying to understand the best way to do things. All I know, is this is used by the debugger, and I also saw it used in GitToolBox plugin.

I guess inlays can also be usefull since they can isolate quite efficiently errors on the same line.

One thing for sure, you can add a background color by using MarkupModel.addLineHighlighter(). Like a said, I tried something which is working well, but the implementation is a bit hacky since you have to link it with each Inlays. When i'll be back home I'll create a PR with everything in it. You are free to merge it, or take inspiration from what was done and improve it.

But in the end, I believe there are only 2 options (I know about) :

chylex commented 1 year ago

I can profile and test it. A PR is appreciated, it will just take potentially a lot of time before it can be released; besides performance and making sure it looks good in various UI themes, I have to consider that some people will want to turn this feature off.

The plugin currently has no configuration. I made it specifically to suit my needs with zero configuration, because I find IntelliJ's configuration to be PITA on both the plugin development and user side. From my experiments with adding configuration for #2, looks like it's still a PITA, but both this and #2 are good reasons to make the plugin configurable, so I will figure something out.

chylex commented 1 year ago

If your PR changes inlays to EditorLinePainter, can you split it into two PRs - one for EditorLinePainter and one for the background color?

synopss commented 1 year ago

My work doesn't use EditorLinePainter. I didn't try to implement it, since I didn't want to change to many things on your work.

chylex commented 1 year ago

The current state looks like this (with some customized inspection severity settings to show all the colors).

obrazek

obrazek

I like it. It solves the issue I observed where people don't see their inspections likely because they aren't prominent enough. I think it's draws attention but isn't so intrusive that I'd need to add configuration to disable it, but will see.

synopss commented 1 year ago

Yeah, I like it too, nice work.

chylex commented 1 year ago

Having issues with ghost highlights that stay after the inspection disappears or moves, don't know how to reliably reproduce it but it's a release blocker until I figure it out.

synopss commented 1 year ago

I remember using a map to link a highlight to an inlay to fix that issue, Since you changed a lot of the code since my poc, maybe you removed that, I am not 100% sure.

chylex commented 1 year ago

There's a map of original highlighters to both inlays and line highlighters. It works for inlays all the time, but for line highlighters it seems to randomly break.

chylex commented 1 year ago

addLineHighlighter doesn't actually work for this, it attaches the highlight to the beginning of the line instead of the area affected by the inspection, and some actions (like auto-import) make them drift apart. It looks like addRangeHighlighter with HighlighterTargetArea.LINES_IN_RANGE might work.