Use line-prefix for embark-completing-read-prompter.

[JimDBh]

Using line-prefix instead of display allows completion packages
like selectrum to highlight the selected candidate and the current
matching characters.

[oantolin]

I don't use the completing-read prompter or Selectrum that often so I hadn't even noticed the problem. This does look nicer, thank you!

[JimDBh]

My pleasure! Thanks for the quick merge :)

[oantolin]

Mmh, looks like line-prefix only works if the string is displayed at the beginning of a line: so it works fine with Selectrum and Icomplete-vertical (at least in their standard configurations), but in icomplete the key bindings aren't visible, and in the *Completions* buffer and in Embark collect grid view only the key bindings for the commands in the left-most column are visible. I guess that's not too bad.

I could go back to using display, but only on the first character (I'd love to use display on an empty string, but text properties need a character to go on). The first character still wouldn't get highlighted in Selectrum, but all the others would.

Or, I could go nuclear, put the the display property on a zero width space or something like that, and then remove that zero width space in a bunch of places: embark-completing-read-prompter itself, an embark transformer for those candidates, and in marginalia. 😝

OK, I probably won't do that last one. I think the realistic options are keep it as it is now, or go back to 'display, maybe just on the first character. I'm leaning towards keeping it as is, so it looks nicer in Selectrum. It's not the end of the world that you don't see the keybindings in icomplete. Having them in the first but not subsequent columns in *Completions* and Embark collect grid view is a little weird, though. I have to decide what's weirder: that or not highlighting the first character in Selectrum...

[JimDBh]

That's indeed some unfortunate surprise. I also thought at first that getting a transform would be the "ideal" way but soon realized it might be too much for this.

Can we add a customizable predicate function here to decide which way to go? For example the users could check for minor modes and/or buffer names to decide whether they should use display or line-predix?

[JimDBh]

After a bit thinking it appears what I suggested probably would not work...

However, looking at the problem, we are basically trying to do two things within one slot: 1) to inform the user of available action shortcuts, 2) to prompt for an action to perform. Would it be a good idea if we separate these?

For example, if a user wants to know what commands are available, we can display a embark collect buffer or maybe something using which-key-mode. But when the user just wants to manually enter a command, we can then use competing read, whether selectrum or icomplete, without showing the shortcuts. In this way we can just use display for displaying the commands, but use the original command name for completing read.

[oantolin]

Well, I like having the key bindings shown in the completing-read prompter because you can use them: if you press @ and then the key binding, it inserts that command at the prompt for you.

But maybe the key bindings should just be in the annotation for the candidates, not in the candidates themselves. I'll try that to see how it feels.

[JimDBh]

Yeah that makes sense. I also played with it a bit. We can add annotation or affixation functions as this, but marginalia would override our annotation or affixation functions. We would need to ask it to respect existing annotations, or add a different category in marginalia.

(if (eq action 'metadata)
              '(metadata . ((category . command)
			    (annotation-function
			     .
			     (lambda (c)
			       (let ((key-alist (cdr (keymap-canonicalize embark--tmp-keymap)))
				     (found nil))
				 (cl-loop
				  for (key . cmd) in key-alist
				  unless found
				  do
				  (if (string-equal (symbol-name cmd) c)
				      (setq found (if (numberp key)
						      (single-key-description key)
						    (key-description key)))))
				 (concat
				  embark-key-action-separator " "
				  (propertize found 'face 'embark-keybinding)))))
			    (affixation-function
			     .
			     (lambda (cands)
			       (mapcar
				(lambda (c)
				  (let ((key-alist (cdr (keymap-canonicalize embark--tmp-keymap)))
					(found nil))
				    (cl-loop
				     for (key . cmd) in key-alist
				     unless found
				     do
				     (if (string-equal (symbol-name cmd) c)
					 (setq found (if (numberp key)
							 (single-key-description key)
						       (key-description key)))))
				    (list c (concat
					     (propertize found 'face 'embark-keybinding)
					     embark-key-action-separator " "
					     ) "")))
				cands)
			       ))))
            (complete-with-action action commands string predicate))

[oantolin]

I decided to not leave (horizontal) Icomplete users out of the keybinding fun. This means going back to display, but now I put in on the last letter, which I think is less noticeable than putting it on the first letter. So Selectrum highlights everything but the last letter and the keybinding. This way looks fine in icomplete, icomplete-vertical, the *Completions* buffer, embark collect both grid and list view, and Ivy. And is only slightly odd looking in Selectrum. Ivy higlights the text in the display property, maybe Selectrum could be changed to do that too?

Also I think it makes more sense to display the command name first and the key binding second, because presumably, if you've forgotten a keybinding, you visually scan for the command name first.

See a4623c3

[JimDBh]

Thanks for letting me know! Yeah this probably is the best with not too many changes.
I'll have a look at selectrum and ivy codes and see how we can highlight the text in display.

[oantolin]

I filed an issue about this: radian-software/selectrum#411.

[oantolin]

I forgot to mention the reason I decided against using the keybindings as annotations. It's because I don't want to loose whatever annotations the user usually has for commands. That's also why I don't make the candidates include an extra space, for example.