Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use full affixation-function annotations in consult--multi #565

Closed
Hugo-Heagren opened this issue Apr 30, 2022 · 23 comments
Closed

Use full affixation-function annotations in consult--multi #565

Hugo-Heagren opened this issue Apr 30, 2022 · 23 comments

Comments

@Hugo-Heagren
Copy link

Hugo-Heagren commented Apr 30, 2022

I would like to be able to return a list from the function used for :annotate in a call to consult--multi, as I currently can with consult--read. There are two reasons:

First, my use-case for consult--multi includes a lot of very long strings. The current annotation mechanism can deliver pretty much everything I need (and does vertical alignment for me, which is great), but vertically aligning the annotations at the end of the longest candidate puts them off-screen to the right. The greater control of affixation-function lets me truncate the candidates themselves too. I know I could build this truncation into the way the list of candidates is generated in the first place, but this seems wrong somehow. I might want to reuse the functions for getting candidate lists elsewhere, and then might need a truncated and non-truncated version for example.

Second, I would like to display some metadata as a prefix to each candidate, which as far as I can tell is currently impossible. (well, I might be able to fiddle manually with display properties, but again, this feels like a bit of a hack).

Is this possible/feasible? Or am I just missing some already existing way of doing this?

@minad
Copy link
Owner

minad commented Apr 30, 2022

Second, I would like to display some metadata as a prefix to each candidate, which as far as I can tell is currently impossible. (well, I might be able to fiddle manually with display properties, but again, this feels like a bit of a hack).

As mentioned in #5 (comment) you can specify prefixes and suffixes with :annotate of consult--read. The only limitations are that you shouldn't modify the candidates themselves (except maybe attaching properties) and that we are operating on individual candidates instead of a list of candidates.

The greater control of affixation-function lets me truncate the candidates themselves too.

I see. Note that truncating the candidates themselves is not supported or support is at least unclear. In principle the API would allow this but the default completion system doesn't handle this well if I recall correctly. Also I am not sure how well Vertico/Selectrum etc handle this. See also minad/vertico#198 where the candidate truncation issue was discussed for recent files.

As of now it is unlikely that I will expose the affixation-function directly by consult--read for various reasons:

  1. The affixation-function API is not well designed. There was some discussion on emacs-devel of replacing this with a better decoration-function but these efforts fizzled. I met fierce resistance when I tried contributing to Emacs, so I gave up on that. I may try again in a few years. ;)
  2. It is unclear if formatting should best be done on the side of the UI, the backend or by some kind of middleware as Marginalia.
  3. The capabilities of the affixation-function are unclear. Does it work if we modify the candidates themselves? This needs an investigation across the existing supported completion UIs (default completion buffer, Mct, Vertico, Icomplete, Selectrum).
  4. Currently the difference is that the :annotate function of consult--read operates on single candidates. I find this convenient and it is not entirely clear to me if you really need the bulk of the candidates to achieve your formatting. Maybe you do - in Marginalia I use the affixation-function to implement left alignment (see https://github.com/minad/marginalia/blob/26f2bd9ee7b63bcad6604108e2f565b34bc6083b/marginalia.el#L1166-L1170). But note that I don't dare to touch the candidates themselves.
  5. I don't need the bulk support of the affixation-function in Consult itself. I am hesitant to extend Consult with capabilities which are not needed by the package itself.

Anyway, feel free to investigate this further! Maybe we come up with some functional proposal. But there are implications across multiple of my packages which need to be sorted out first. This is definitely a downside of the approach of relying on existing APIs and trying to find a common denominator. It can happen that you hit the limitations (or under specifications) of the existing APIs and then there is not much we can do.

@minad minad closed this as completed Apr 30, 2022
@minad
Copy link
Owner

minad commented Apr 30, 2022

I forgot to mention the most important argument why I am against truncating candidates - my reasoning is that the candidate itself is always more important than the annotation. This means if we are out of space it is always better to truncate the annotation. You may disagree with me on that (I think @jdtsmith also disagreed on that in minad/vertico#198), but that's at least my justification to not touch or truncate the candidates, besides the technical issues mentioned before, which may or may not hold. The candidate is the string we are matching against and the candidate is highlighted by the completion style.

@jdtsmith
Copy link
Contributor

jdtsmith commented Apr 30, 2022

the candidate itself is always more important than the annotation

I agree with this in almost all contexts, except one. IMO the long path of a recentf file candidate doesn't deserve that full protection. The path of a buffer file is part of its annotation, and find file visits only a single path at a time. So only for recentf files is the full path considered "part of the candidate".

I use this as arg-filter advice to vertico--format-candidate, to truncate long file candidates on display only:

(defun my/vertico-truncate-candidates (args)
    (if-let ((arg (car args))
	     (type (get-text-property 0 'multi-category arg))
	     ((eq (car-safe type) 'file))
	     (w (max 30 (- (window-width) (consult--display-width (nth 2 args)))))
	     (l (length arg))
	     ((> l w)))
	(setcar args (concat "" (truncate-string-to-width arg l (- l w)))))
    args)

The fact that the candidate is modified for display only at the last minute means you can still match text which is truncated/not displayed (which is either a feature, or confusing depending on your outlook). But it keeps annotations nicely aligned:

image

I do wish vertico would reformat the candidates for display on frame size changes, since an easy way to see more is to expand the frame. For now I add+remove a character from the search to force reformatting.

@Hugo-Heagren
Copy link
Author

@jdtsmith that's a very elegant solution -- I really like it. I might allow for something similar in my project.

@minad
Copy link
Owner

minad commented Apr 30, 2022

I think you can always overwrite/modify the candidate with a display/invisble property, e.g., for recent files it should be possible to make the path of the file invisible and only keep the file name. This would allow you to shorten the candidates but of course it wouldn't auto resize depending on the window width. But maybe you could achieve the same with the affixation function by modifying the candidate there (not the string itself), but only propertizing it. Then it would even work dynamically depending on window width since the affixations are recomputed by vertico in the post-command-hook. Maybe then it would be needed to open up the consult--read API to support lists of candidates.

@minad
Copy link
Owner

minad commented Apr 30, 2022

Btw, I just realized that consult--multi-annotate does not support prefix annotations and I tried to fix this in 349d31d. I had remembered this wrongly when I responded with #565 (comment).

The issue here is that we break the alignment then. Therefore I reverted this for now. This was probably the reason why I originally didn't support affixations for consult--multi. The issue is that we want to allow composition of multi sources and still align the annotations somewhat nicely. I think there is no good general solution there and I guess we have to live with what we have right now. There is also a point in keeping things simple.

Any proposals how this could be solved? The answer is probably that the UI should handle this, but this is out of the question due to the weakness and limitations of the affixation API. This issue won't be solved without introducing a better API which pushes the presentation entirely to the UI (the aforementioned hypothetical decorations API).

@Hugo-Heagren
Copy link
Author

Hugo-Heagren commented Apr 30, 2022

Any proposals how this could be solved?

My first intuition is that it could be handled similarly to the current relationship between candidates and annotations which appear to their right: by having an alignment string interpolated between them. So consult--multi-annotate would have to take another argument prefix-align, and this would have to calculated inside consult--multi, as align is at the moment. I don't know much about display properties and aligning, so maybe this wouldn't work?

EDIT: had a look at this and I can't see how a similar approach to the alignment we use already could work, without either:

  • generating and examining every prefix annotation
  • setting constraints on what those annotations should be

Neither is a great approach. To be honest though, in other areas of emacs, alignment like this is left up to the author of the particular function (that is, alignment should be done by making all the annotations the same length). Indeed, it is even up to the author whether they want alignment (sometimes they don't. See the binding annotations in execute-extended-command). I would be happy with such a requirement in prefix annotations in consult--multi. I suppose those who didn't like it could just not use it until a better solution is found?

@minad
Copy link
Owner

minad commented Apr 30, 2022

Hmm, we could use an affixation function which takes a list and then computes the alignment for all the candidates. This would also work for consult--multi, but then we would have to add full affixation support to consult--read. But I must say I am also not a fan of prefixes anyway. Prefixes seem mostly useful for icons but not for many other things.

@oantolin
Copy link
Contributor

Prefixes seem mostly useful for icons but not for many other things.

I've heard of people using them for line numbers... 🙄

@Hugo-Heagren
Copy link
Author

Prefixes seem mostly useful for icons but not for many other things.

My intention was time-length strings (e.g. 00:10, 02:10:22).

@Hugo-Heagren
Copy link
Author

Hmm, we could use an affixation function which takes a list and then computes the alignment for all the candidates. This would also work for consult--multi, but then we would have to add full affixation support to consult--read

I would certainly be happy with this as a solution, though I don't know enough (yet!) about consult's internals to know how to implement it. It would work perfectly well though.

@minad
Copy link
Owner

minad commented Apr 30, 2022

@oantolin

Oh, wait, I misremembered, line numbers in consult-line are line-prefix properties on candidates, not handle by an affixation function. Never mind.

consult-line uses affixation prefixes for line numbers :)

@oantolin
Copy link
Contributor

consult-line uses affixation prefixes for line numbers :)

@minad Damn, you saw my comment before I could delete it.

@minad
Copy link
Owner

minad commented Apr 30, 2022

@Hugo-Heagren

I would certainly be happy with this as a solution, though I don't know enough (yet!) about consult's internals to know how to implement it. It would work perfectly well though.

Okay, I guess I will bite the bullet and implement alignment directly in consult--read. Then we can keep the calling convention as is but annotations and affixations will become automatically aligned.

@Hugo-Heagren
Copy link
Author

Okay, I guess I will bite the bullet and implement alignment directly in consult--read. Then we can keep the calling convention as is but annotations and affixations will become automatically aligned.

This would be absolutely amazing. I'm happy to help in any way if I can, but I'm sure you'll do a better job than I could.

@minad
Copy link
Owner

minad commented Apr 30, 2022

Okay, I tried this in a separate branch 8504f10. The result is not satisfactory at all. It doesn't simplify a bit for the existing Consult :annotate functions, since we still have to align candidates manually, if we don't want to have ugly jumping during filtering. I had to jump through many hoops to make the automatic alignment work half-decently in Marginalia (temporarily remembering the maximum candidate width so far in a buffer local minibuffer variable etc). This is complexity I don't want to have here. Therefore I have to say no to this feature request. No auto alignment and thus no prefix support for multi sources. 🤷

@minad
Copy link
Owner

minad commented Apr 30, 2022

There is one compromise we could make - we could still support prefixes and suffixes, I mean just pass them through and leaving alignment up to the user (as was suggested in #565 (comment)). But this will potentially lead to an ugly result in the end and hurt composability of sources.

I must admit, I don't find the use case in #565 (comment) very convincing. Why not put the time stamp in the suffix? I kind of like that we have suffixes only and the candidates in the front. There are a few special exceptions like icons, line numbers in consult-line etc. Maybe the short time stamp in a music player is a similar special case?

@minad
Copy link
Owner

minad commented Apr 30, 2022

The only way out I am seeing is via a new API which moves all the alignment work on to the frontend. If someone proposes and implements a patch in Emacs itself (the aforementioned decoration-function), I am happy to support it in Vertico and use it Consult and Marginalia 😛

Let me summarize the situation:

  • Marginalia uses fixed (hard-coded) alignment for the annotation fields. This is okay, but would better be if the frontend UI receives the columns and aligns them for us.
  • Marginalia dynamically left-aligns the annotation suffixes by remembering the maximum candidate width so far etc. This is a big mess and should definitely go better in the frontend.
  • Marginalia also supports right-alignment and center-alignment of the suffix annotations.
  • Marginalia truncates fields based on the minibuffer window width, which is just a good guess. Marginalia doesn't know where the annotations are actually displayed, e.g., Embark collect buffers.
  • The frontends in contrast have knowledge of the display width etc.
  • The frontend Vertico doesn't support candidate truncation, but maybe it should as has been argued (Use full affixation-function annotations in consult--multi #565 (comment)).
  • Consult manually aligns annotations since it knows the maximum candidate width in those cases. I don't want to duplicate the same complicated auto alignment from Marginalia.
  • Embark collect doesn't handle the Marginalia annotation alignment particularily well, since it puts the annotations and candidates in tabulated list columns (see Improve MX marginalia alignment in embark collect oantolin/embark#445). What Embark does should be perfectly justified if the alignment responsibility is pushed to the frontend.
  • Most Emacs builtin functions don't align at all, neither the prefixes nor the suffixes.

All these points hint at that we should move the entire alignment and truncation business to the frontend instead of scattering across backends and middleware. Advanced frontends (e.g. Embark collect tabulated list mode) would in principle even let the user resize columns.

I should add, the compromise #565 (comment) is still something I would find at least somewhat acceptable. It leaves the principle intact, that consult--read is a thin layer over completing-read. While composition of sources is hurt in general, when composing sources the user could in principle replace or adjust the annotation functions to leave the manual alignment intact. In practice the set of sources which are composed is probably limited anyway as in @Hugo-Heagren's consult-emms use case.

@jdtsmith
Copy link
Contributor

Prefixes seem mostly useful for icons but not for many other things.

My intention was time-length strings (e.g. 00:10, 02:10:22).

But it seems that those types of prefixes should be quite straightforward to format at a constant width?

@jdtsmith
Copy link
Contributor

All these points hint at that we should move the entire alignment and truncation business to the frontend

This seems eminently sensible. So just pass the display front end a vector of n parts, and some instructions about how to align them (minimum-width, right-/left-/center-align, truncate-from-left/truncate-from-right/no-truncate)? But if only the currently displayed rows are passed to the front-end, there will still be sliding around as you scroll I'd guess. And presumably passing all of them to compute stable alignment up front would be expensive, though you could have a cutoff at 500 or whatever.

What if alignment "suggestions" could be encoded as special text properties in the string, for the front end to respect or just ignore? These properties could indicate column breaks, alignment, min/max width, truncation, etc. That way non-compliant front-ends would still display normally.

@minad
Copy link
Owner

minad commented Apr 30, 2022

@jdtsmith

But it seems that those types of prefixes should be quite straightforward to format at a constant width?

Of course. The timestamps are constant width. The issue is that you cannot compose different sources with different prefix width. The solution I tried above was to dynamically compute the maximum width and then align, but this has many disadvantages.

So just pass the display front end a vector of n parts, and some instructions about how to align them (minimum-width, right-/left-/center-align, truncate-from-left/truncate-from-right/no-truncate)?

Yes, exactly. That's more or less what I had in mind for the decoration-function. The suggestions could either be implemented as additional plist properties or text properties. There are many options. Text properties have the disadvantage that these require you to mutate/propertize the strings.

One could start implementing an x-decoration-function extension in Vertico and use it in Marginalia. But at some point this is growing over the top since we would need fallbacks for the annotation-function and the affixation-function. One could also consider implementing a middleware which sits between Vertico and Marginalia/Consult which translates x-decoration-function to the affixation function.

@jdtsmith
Copy link
Contributor

jdtsmith commented May 1, 2022

Do you mean fallbacks in case they attempt their own alignment? I always thought having annotation/affixation try to achieve alignment was a violation of separation of content and presentation. Like an HTML table cell setting its own width.

@minad
Copy link
Owner

minad commented May 1, 2022

No, I mean fallbacks for UIs which don't implement the more advanced API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants