minad / consult

:mag: consult.el - Consulting completing-read
GNU General Public License v3.0
1.14k stars 99 forks source link

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

Closed Hugo-Heagren closed 2 years ago

Hugo-Heagren commented 2 years ago

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 commented 2 years ago

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 https://github.com/minad/consult/issues/5#issuecomment-1113972233 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 https://github.com/minad/vertico/issues/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 commented 2 years ago

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 https://github.com/minad/vertico/issues/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 commented 2 years ago

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 commented 2 years ago

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

minad commented 2 years ago

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 commented 2 years ago

Btw, I just realized that consult--multi-annotate does not support prefix annotations and I tried to fix this in https://github.com/minad/consult/commit/349d31dbc6ce2120f256ea07f2772ceb0df3522d. I had remembered this wrongly when I responded with https://github.com/minad/consult/issues/565#issuecomment-1113984780.

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 commented 2 years ago

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:

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 commented 2 years ago

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 commented 2 years ago

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

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

Hugo-Heagren commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

consult-line uses affixation prefixes for line numbers :)

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

minad commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

Okay, I tried this in a separate branch https://github.com/minad/consult/commit/8504f10f652f5d4dc88c596422b6e81d84eae28a. 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. :shrug:

minad commented 2 years ago

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 https://github.com/minad/consult/issues/565#issuecomment-1114021844). 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 https://github.com/minad/consult/issues/565#issuecomment-1114025032 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 commented 2 years ago

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 :stuck_out_tongue:

Let me summarize the situation:

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 https://github.com/minad/consult/issues/565#issuecomment-1114045201 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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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