company-mode / company-quickhelp

Documentation popup for Company
GNU General Public License v3.0
374 stars 33 forks source link

Added `company-quickhelp-face' and modified company-quickhelp--show #103

Closed Dr-Dd closed 4 years ago

Dr-Dd commented 4 years ago

I don't know if this is the correct way to solve this problem, but i implemented it in a way so that the spacing and the tooltip position remains correct, also the option is available only if you set to true company-quickhelp-use-propertized-text. I implemented it as suggested by pos-tip-show-no-propertize documentation. In theory if i understand correctly pos-tip should have its own face named pos-tip, but at the moment it lacks such feature. If it existed, i would have put the created company-quickhelp-face to inherit from pos-tip, instead of the tooltip (which right now is the most correct approach IMO). If pos-tip had a face, i also imagine it would inherit it from tooltip since fg and bg properties are already inherited from tooltip.

Anyway this is more of a proposition to fix the need to have the possibility to customize the pos-tip quickhelp tooltip face (as seen in #98, #85, #81, #53, #50, #36). Tell me if you agree with the changes.

I might be missing some logic behind the way the propertization is handled right now, so if there are any issues with this implementation let me know.

(probably the program should be reworked to use the newly created faces)

Dr-Dd commented 4 years ago

This works, but not all the time. The problem is that, even though we are using propertized text, the pos relative to when the width/height exceeds the frame size is still related to the frame-char-size, and not the propertized text size. I don't know the logic well enough to rework this.

In the meantime i fixed #96. I'm gonna open another pull request with it, and i'm gonna rebranch this one.

dgutov commented 4 years ago

I don't know if this is the correct way to solve this problem, but i implemented it in a way so that the spacing and the tooltip position remains correct

That was a good effort, but I'd need to see a more detailed description of the problem first.

As far as the change itself, I would probably be okay with it.

Dr-Dd commented 4 years ago

I'll try to iron out the aforementioned problems, then i'll send a pull request.

Any indication to how the quickhelp tooltip gets repositioned when the doc size is too wide or high for the buffer? That's the thing that i'm trying to fix, but i can't seem to find where this adjustment happens.

dgutov commented 4 years ago

That probably happens in pos-tip. We tell it to show a tooltip of width W at position X-Y, and it does its best.

I think that should be handled here instead, and the width we pass to pos-tip should be reduced when we know it can't fit in the current frame.