search

eclipse-platform / eclipse.platform.text

8 stars 45 forks source link

CompositeInformationControl should use its controls to compute the size #199

Closed ddscharfe closed 1 year ago

ddscharfe commented 1 year ago

We are using a generic editor, which registers a hover provider (org.eclipse.ui.genericeditor.hoverProviders). This leads to the creation of a CompositeInformationControl when hovering. However the hover looks to small.

I will contribute a PR for this.

See https://github.com/eclipse-cdt/cdt-lsp/issues/70

iloveeclipse commented 1 year ago

See eclipse-cdt/cdt-lsp#70

I can't see there anything about wrong size, only about wrong content.

Can you please provide some hints where the code is that creates hover that "looks to small"? Ideally a self containing example project.

ddscharfe commented 1 year ago

You cannot se this in the linked issue, because the issue complains about the content. I fixed that by adding a hover provider, which results in a hover that looks like this: image

This is how it looks with the PR: image

mickaelistria commented 1 year ago

@iloveeclipse This is overall a noticeable and safe and isolated enhancement but the current timeframe seems to indicate that there is no chance this gets shipped before long; however the value of this fix is pretty high. I would like to merge it ASAP so it makes it in 4.29 M3. What do you think?

iloveeclipse commented 1 year ago

I'm OK if you have tested it / verified with the IDE. I've dismissed my review because I had no time to do that.

mickaelistria commented 1 year ago

I have tested it quickly, but will spend more time on it right now to do intensive testing; and unless I notice some new annoyance with the patch, I'll mege.

ddscharfe commented 1 year ago

I have tested it quickly, but will spend more time on it right now to do intensive testing; and unless I notice some new annoyance with the patch, I'll mege.

I'll have a look at the things you mentioned in #200 right now, so maybe you want to wait before testing?

mickaelistria commented 1 year ago

@ddscharfe If you think you can have them covered within a few hours, I can wait; otherwise we can go on the current patch and make enhancement in future changes. FWIW, there is some pressure because we're starting the code freeze period before 4.28 release.