[POC] Provide support for candidate prefix.

[yyoncho]

• this can be used for showing icons in company-posframe

• Open question: how to pass :x-pixel-offset to company-posframe?

Here it is demo how it looks like with company-posframe.

[dgutov]

Hi! This is really for icon support, right?

I think I'd prefer if backends didn't choose individual icons, but rather returned some "type" values, which would correspond to icons from a shared registry. company-box has something like this, but we could incorporate a similar registry into the core.

This way the odds of having different backends use totally different icons become rather small. And the centralized code could fall back to ASCII when run in the terminal, for instance.

There have been a few discussions about this feature here. Perhaps you would like to pick up from where #756 (comment) stopped?

Open question: how to pass :x-pixel-offset to company-posframe?

Can't answer that offhand, but first I'd need to understand why this is needed.

[yyoncho]

I think I'd prefer if backends didn't choose individual icons, but rather returned some "type" values, which would correspond to icons from a shared registry.

Will introducing a defvar which can be used like - (funcall company-resolve-icon-function :kind 'class :selected t) and changing backends to return kind be sufficient for you? kind is the LSP's set of kinds which is fairly generic.

Can't answer that offhand, but first I'd need to understand why this is needed.

It is needed for the proper placement of the popup, it is calculated like that right now:

           :x-pixel-offset (* -1 company-tooltip-margin (default-font-width))

With the design including company-resolve-icon-function we could have another defvar for calculating the offset.

[dgutov]

Will introducing a defvar which can be used like - (funcall company-resolve-icon-function :kind 'class :selected t) and changing backends to return kind be sufficient for you?

It will still need some decent default configuration that works on free systems.

I don't want this feature to only be available with select backends. Or have different backends enforce different configurations.

It is needed for the proper placement of the popup, it is calculated like that right now:

I see. Well, there is no ideal way if the icons are in different width (company-posframe wouldn't be able to support this). But otherwise it can be done, one way or another. It would require an update to its code, that's all.

[yyoncho]

AFAIK there are at least 3 working open-source emacs solutions providing icons and neither of them is Gnu ELPA compatible. On lsp-mode side we already have a centralized registry with icons and it is used across all(?) of the components - symbol browser, imenu, header integration, error list, project explorer, etc so even if I implement something in company-mode I still won't use it.

[dgutov]

there are at least 3 working open-source emacs solutions providing icons and neither of them is Gnu ELPA compatible

What are the problems there?

so even if I implement something in company-mode I still won't use it

That's okay, as long as your users can.

[dgutov]

That's okay, as long as your users can.

And as long as the users can switch between different sets of icons at will. Use the default icons with lsp-mode, or your icons with Eglot, or with all other non-LSP backends as well.

See, if there is no built-in registry, then even the kind values don't have any meaning. I can't document them. And other backends can't use them consistently.

[yyoncho]

What are the problems there?

Let's take https://github.com/domtronn/all-the-icons.el - how can I use it in company-mode as icon provider? Can I go in company-box and move the mapping kind->icon in company-mode?

See, if there is no built-in registry, then even the kind values don't have any meaning. I can't document them. And other backends can't use them consistently.

The proposed fix is generic and allows altering the prefix of the candidate which might or might not be related to icons - this can be documented and have meaning.

[dgutov]

Let's take https://github.com/domtronn/all-the-icons.el - how can I use it in company-mode as icon provider?

I suppose there is no way. The primary author has not signed the copyright papers. But the elisp code itself is relatively simple, so it could be rewritten by someone motivated. The primary difficulty is the availability of fonts. But hey, if the licenses are good, we could make this feature optional and reliant on fonts being installed.

The package is pretty big and well-designed, so I agree if it seems wasteful to reimplement it.

Can I go in company-box and move the mapping kind->icon in company-mode?

This seems closer to the MVP approach because it includes a set of images to use if all-the-icons is not available. And the number of contributors is smaller. So you could "rewrite" the code (I don't know how much Elisp is really needed, and some parts would be obviously hard to express in a different way), or you could not bother with that and ask for the people who touched the icon-mapping code to sign the papers. I don't know how many there are who made non-trivial changes. Probably not many.

But the origin of the images would need to be determined first.

The proposed fix is generic and allows altering the prefix of the candidate which might or might not be related to icons - this can be documented and have meaning.

Generic prefix with arbitrary contents?

[yyoncho]

The primary author has not signed the copyright papers.

It is MIT licenced so copyright assignment it won't be sufficient either way.

But the origin of the images would need to be determined first.

AFAIK it is from vscode original repo which is with MIT license. I am not sure if elpa accepts mit licenced artwork.

Generic prefix with arbitrary contents?

Why not? It is symmetric to annotation.

[dgutov]

It is MIT licenced so copyright assignment it won't be sufficient either way.

A copyright assignment means that the other party is free to release the work under any other license. Including GNU GPL, which is the custom here. But MIT is hardly problematic.

I am not sure if elpa accepts mit licenced artwork.

Most likely yes. Emacs does, too. There was a recent discussion on emacs-devel about it.

Why not? It is symmetric to annotation.

I'm not crazy about people putting arbitrary things in annotation either.

But it's one thing to have one arbitrary field. It's entirely another to have two of them, so some people might decide to put everything in the prefix, for instance.

And how did you want to define it anyway, in ways that both allowed to show backend-independent icons, as well as arbitrary text or other decorations?

[yyoncho]

I am not sure if elpa accepts mit licenced artwork.

Most likely yes. Emacs does, too. There was a recent discussion on emacs-devel about it.

Cool, so you are fine with including the vscode icon set here and implementing a function which maps kind to the icon? FWIW it will still require capable frontend like company-posframe to use the images and additional activation code and it won't work OOTB.

And how did you want to define it anyway, in ways that both allowed to show backend-independent icons, as well as arbitrary text or other decorations?

AFAICS from the code if you concat both and then pass them as left margin it will work. But it all depends on the backend.

[dgutov]

@yyoncho So you think there is no reasonable way to make it work with the default popup?

[yyoncho]

@yyoncho So you think there is no reasonable way to make it work with the default popup?

I am not aware of a reasonable solution given the elpa limitations.

As a side note, passing the candidate to company-space-string as an optional argument and slight restructuring of the code to allow having different left and right martin will work for me too.

[dgutov]

What ELPA limitations?

We've just discussed that using VS Code's pictures is fine. MIT-licensed code too.

As a side note, passing the candidate to company-space-string as an optional argument and slight restructuring of the code to allow having different left and right martin will work for me too.

Refactoring the tooltip code for better reuse from outside could be done. If that's what you are referring to.

[yyoncho]

I somehow lost my comment...

I realized that there is no need to change company-mode since it is already providing the extension points that are needed for the icons and for the other things that that are too lsp-mode specific to get to the core.

(advice-add 'company-fill-propertize :around #'lsp--company-fill-propertize)

(defun lsp--company-fill-propertize (orig value annotation width selected left right)
  (funcall orig value (when selected annotation) width selected
           (or (lsp--candidate-prefix value selected) left) right))

What ELPA limitations?
We've just discussed that using VS Code's pictures is fine. MIT-licensed code too.

I was referring to the fact that there is no reasonable way to use all-the-icons as company-mode's default icon provider.

[dgutov]

OK then.

That was disappointing.

[yyoncho]

I thought that this will be better for you. I mean, either way, with company-mode restricted not to use melpa packages half of the solution would be in elpa and the other half will be in melpa(the most common one which uses traditional company-tooltip). I can add method get type and a function to retrieve the icons, it is trivial to do so but I am not sure if this will help company-mode project.

[dgutov]

If the "half" in ELPA is functional and shows the images (even if not the ones you personally want to see), it can be a good solution.

Having backends talk to frontends via some undocumented protocol is not so great.

[yyoncho]

If the "half" in ELPA is functional and shows the images

AFAICS company-posframe is not in elpa so the elpa part won't be functional. Up to my knowledge, the tooltip frontend cannot display images and at the same time, there are no font icons in elpa.

[dgutov]

tooltip frontend cannot display images

I'm fairly confident that is can (Emacs itself can display images).

The main problems with that will be image sizing and the behavior in non-graphical terminals. But we could find some acceptable tweaks for both.

[yyoncho]

I'm fairly confident that is can (Emacs itself can display images).

Do you have a recipe for that? Because ATM when I run the default frontend with images I don't see the images but empty space while it works fine with all-the-icons.

[dgutov]

Indeed, there are some problems. You can try this out (update the file name according to your system):

diff --git a/company.el b/company.el
index d1f17fe..8fe9221 100644
--- a/company.el
+++ b/company.el
@@ -2532,6 +2533,16 @@ If SHOW-VERSION is non-nil, show the version in the echo area."
     (apply 'concat pieces)))
 
 (defun company-fill-propertize (value annotation width selected left right)
+  (let* ((spec (list 'image :file
+                     "~/.emacs.d/site-lisp/company/Misc.png"
+                     :type 'png :scale 1
+                     :max-height (default-font-height)
+                     :ascent 'center))
+         (is (image-size spec)))
+    (setq left
+          (concat
+           (propertize " " 'display spec)
+           (propertize " " 'display `(space . (:width ,(- 2 (car is))))))))
   (let* ((margin (length left))
          (common (or (company-call-backend 'match value)
                      (if company-common
@@ -2713,7 +2727,7 @@ If SHOW-VERSION is non-nil, show the version in the echo area."
     ww))
 
 (defun company--replacement-string (lines old column nl &optional align-top)
-  (cl-decf column company-tooltip-margin)
+  (cl-decf column (1+ company-tooltip-margin))
 
   (when (and align-top company-tooltip-flip-when-above)
     (setq lines (reverse lines)))
@@ -2793,6 +2807,7 @@ If SHOW-VERSION is non-nil, show the version in the echo area."
           lines-copy lines)
 
     (cl-decf window-width (* 2 company-tooltip-margin))
+    (cl-decf window-width 1)
     (when scrollbar-bounds (cl-decf window-width))
 
     (dotimes (_ len)
@@ -2829,7 +2844,7 @@ If SHOW-VERSION is non-nil, show the version in the echo area."
                (str (car item))
                (annotation (cdr item))
                (margin (company-space-string company-tooltip-margin))
-               (left margin)
+               (left (company-space-string (1+ company-tooltip-margin)))
                (right margin)
                (width width))
           (when (< numbered 10)
@@ -2958,10 +2973,10 @@ Returns a negative number if the tooltip should be displayed above point."
       (overlay-put ov 'priority 111)
       ;; No (extra) prefix for the first line.
       (overlay-put ov 'line-prefix "")
-      ;; `display' is better
-      ;; (http://debbugs.gnu.org/18285, http://debbugs.gnu.org/20847),
-      ;; but it doesn't work on 0-length overlays.
-      (if (< (overlay-start ov) (overlay-end ov))
-          (overlay-put ov 'display disp)
-        (overlay-put ov 'after-string disp)
-        (overlay-put ov 'invisible t))
+      (overlay-put ov 'after-string disp)
+      ;; `display' is better than `invisible':
+      ;; https://debbugs.gnu.org/18285
+      ;; https://debbugs.gnu.org/20847
+      ;; https://debbugs.gnu.org/42521
+      (overlay-put ov 'display "")
       (overlay-put ov 'face 'default)
       (overlay-put ov 'window (selected-window)))))
 

And (setq company-tooltip-margin 2).

If this direction looks interesting, though, we can find out whether something could be done so that the second part of that diff is unnecessary (because simply applying it would bring back problems described in bug reports in the comment above).

EDIT: Updated the diff above a little (twice).

[dgutov]

A screenshot: https://imgur.com/a/cJIUuvn

(It's huge; it's a GNOME bug, apparently).

[ericdallo]

It looks amazing on non GNOME(bpswm):

[yyoncho]

I tested it, and my PR works fine after applying only the overlay/display part of the patch. I am not sure if the rest of the patch is related to the placement of the popup.

What are the options now? I don't quite understand this code.

[dgutov]

It's a snippet that adds an image to the left of every item in the default completion popup. It needs a path to an existing image file, though.

That's what you asked for, right?

Of course, adding icons to the popup this way conflicts with your patch. From it, I would only take the changes to company-capf.el.

[yyoncho]

My question is about handling this:

       ;; but it doesn't work on 0-length overlays.
-      (if (< (overlay-start ov) (overlay-end ov))
-          (overlay-put ov 'display disp)
+      ;; (if (< (overlay-start ov) (overlay-end ov))
+      ;;     (overlay-put ov 'my-display disp)
         (overlay-put ov 'after-string disp)
-        (overlay-put ov 'invisible t))
+        (overlay-put ov 'invisible t);; )

My patch(or another patch) could be easily adjusted to call backend for candidate type, resolve the corresponding icon in company-mode via customizable function, replacing left with it and it will just work with company-posframe but it won't work with company-tooltip without the quoted diff.

[dgutov]

I've filed http://debbugs.gnu.org/cgi/bugreport.cgi?bug=42521, let's see how it goes.

[dgutov]

Now what?

We can wait for a few days.

Or go ahead with the text-based approach (either of them), keeping in mind that the default implementation could be upgraded to use images later. The downside would be that the images seem to be the more complicated case, so supporting them out-of-the-box should help testing the capabilities of the new facility to a larger degree.

If this is not the case for tooltip frontend IMO we should still have the option to run with 1 column margin for posframe frontends.

I'm sure this is doable, one way or another. But note that currently company-posframe doesn't customize any of the line rendering logic. It is reused fully from the default popup.

[yyoncho]

Here it is almost everything glued together: https://github.com/emacs-lsp/lsp-ide/blob/Demo/init.el

And the icons part from company-mode yyoncho@a977d8f

[dgutov]

Thanks!

I got this backtrace:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  expand-file-name(nil "/tmp/new/company-20200729.2244/icons/vscode-dark/")
  company-make-icon#'"#32302f"
  company-vscode-icons-margin-function#'nil
  company--create-lines(0 10)
  company-posframe-show()
  ...

Apparently, the mapping was missing this: (function . "symbol-method.png").

And I see you went with the margin=2 approach. Any plans to change that?

And another question: I see the string "vscode-dark". Does it have a different set of icons for light themes?

[yyoncho]

Thanks!

I got this backtrace:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  expand-file-name(nil "/tmp/new/company-20200729.2244/icons/vscode-dark/")
  company-make-icon#'"#32302f"
  company-vscode-icons-margin-function#'nil
  company--create-lines(0 10)
  company-posframe-show()
  ...

Apparently, the mapping was missing this: (function . "symbol-method.png").

Added the mapping and a check to avoid the error.

And I see you went with the margin=2 approach. Any plans to change that?

I did that just to make it work and because I was unable to find the method to measure a string in pixels without string being inserted in a buffer in order to the proper placement of the popup. And also, that change doesn't require a change in the frontend.

FTR, in the latest code I changed the image properties. The one suggested by you were causing blurry image at least with posframe.

 :width 15
 :height 15

And another question: I see the string "vscode-dark". Does it have a different
set of icons for light themes?

Yes. If we are going to use these icons we will probably need to replicate that.

[dgutov]

I was unable to find the method to measure a string in pixels without string being inserted in a buffer in order to the proper placement of the popup

All right. It's fine as the first version. Just curious where do we go from here.

The frontend-specific code issues aside, some arithmetics could be done based on the image width and the width of the default font. Or the offset could be indicated to the frontend by the presence of some text property inside the string. Then the frontend will just need to measure the width of the prefix that lacks said property.

FTR, in the latest code I changed the image properties. The one suggested by you were causing blurry image at least with posframe

Where's the latest code?

Technical issues aside, I just checked, and you don't seem to have copyright assignment on file. Would you like to get started on that?

[yyoncho]

I am pushing here - yyoncho@269e358

Technical issues aside, I just checked, and you don't seem to have copyright assignment on file. Would you like to get started on that?

I will try to do that this week.

[dgutov]

I am pushing here - yyoncho/company-mode@269e358

👍

one suggested by you were causing blurry image at least with posframe

This will require some further investigation, I think. I didn't see any blurriness myself, and I tried it with the posframe fe. I also have a hidpi screen; the configured fonts looked pretty small.

Another thing to try is call create-image instead of constructing the spec manually as I did. The return value usually applied some specific :scale. I'm not sure how it is determined.

I will try to do that this week.

Do you have the form?

[yyoncho]

This will require some further investigation, I think. I didn't see any blurriness myself, and I tried it with the posframe fe. I also have a hidpi screen; the configured fonts looked pretty small.

Hm, maybe I do not see the blurriness due to 15px being smaller than your version. But definitely they are more blurry than what vscode shows. VScode uses svg. Is svg on the table for us? I have some trouble compiling my emacs with svg support but I guess it may yield better looks. But I think that I saw someone complaining from svg support in emacs(but I might be wrong).

I will try to do that this week.

Do you have the form?

Yeah, I am on step 2.

Regarding our reddit conversation, here it is everything glued together (with few local patches):

https://www.youtube.com/watch?v=7BODBLwgw1s&feature=youtu.be

[dgutov]

maybe I do not see the blurriness due to 15px being smaller than your version

I think that depends on the actual dimensions of the images, as well as your font sizes. It's hard to discuss that without pictures, as well as when custom configurations come into play.

Is svg on the table for us?

It's an option. It will reduce the pool of users who are able to enjoy the feature, so a fallback could be useful. I'd have to see/experience the blurriness you talk about, to have a solid opinion. SVG example from https://www.gnu.org/software/emacs/manual/html_node/elisp/SVG-Images.html seems to work fine here, but I'd have to see how well it supports externally made images.

Regarding our reddit conversation, here it is everything glued together (with few local patches):

Looking very nice.

Is the tooltip frontend used there company-box? I don't see the same two-column margins.

What are the odds of the calltips feature making it into GNU ELPA?

[yyoncho]

Is the tooltip frontend used there company-box? I don't see the same two-column margins.

this is company-posframe and it uses two-column margins - I will update init.el from lsp-ide package to work like that.

What are the odds of the calltips feature making it into GNU ELPA?

I don't see anything appropriate for elpa considering the info is coming from lsp server and the popup handling code is like 10 lines of code, can you clarify?

[dgutov]

this is company-posframe and it uses two-column margins

Ah, I guess the difference in looks is due to your font size. After all, fonts are scalable, whereas a png has a fixed size. So it works better than expected, even.

I don't see anything appropriate for elpa considering the info is coming from lsp server and the popup handling code is like 10 lines of code, can you clarify?

To create an alternative for Eldoc. Eldoc's current most popular use is to show a function/method signature, but a specialized kind of indicator can work better (and with better spatial locality). With a generalized hook for other packages to plug into, not just LSP.

Even if it's only 10 lines long (though, generalized, it will become a little longer), as long as these lines were written with some care for the UI, it can be quite valuable.

[yyoncho]

as long as these lines were written with some care for the UI, it can be quite valuable.

TBO I don't see anything valuable, here it is the code.

(defun lsp-signature-posframe  (str)
  (if str
      (posframe-show "*lsp-signature*"
                     :position (point)
                     :string str
                     :poshandler 'posframe-poshandler-point-bottom-left-corner-upward
                     :background-color (face-attribute 'tooltip :background)
                     :height 7
                     :width 60
                     :min-width 60)
    (posframe-hide "*lsp-signature*")))

;; config
(setq lsp-signature-function 'lsp-signature-posframe)

[dgutov]

I see what you meant by it being very brief, but it's a good start. It is triggered on post-command-hook, right? And shown whenever the language server provides a non-empty response?

I would probably like to have this feature when editing Elisp as well (and with my own IDE-ish package, too). And I would probably like for all versions to use the same faces. And support HiDPI screens. Anyway, good to have some code to play with.

[yyoncho]

It is triggered on post-command-hook, right?

Yes.

And shown whenever the language server provides a non-empty response?

No - there are triggerChars in the spec which activate the feature.

[dgutov]

there are triggerChars in the spec which activate the feature

Same trigger chars that initiate automatic completion?

[yyoncho]

Same trigger chars that initiate automatic completion?

No, separate one specific for signature help. (e. g. (, ,).

[dgutov]

Thanks. When is it hidden? After the first time the request returns nil?

[yyoncho]

Thanks. When is it hidden? After the first time the request returns nil?

This is what we are doing right now. But I havent crosschecked with vscode.

[dgutov]

As an aside, here are some alternative visuals: https://blog.atom.io/2015/05/15/new-autocomplete.html

I think the images at the end are pretty compelling. Though they probably don't fit the data provided by LSP.

[dgutov]

@yyoncho I've pushed to master the updated version of the patch that makes possible rendering images in the overlay popup.