Revise x-group-function, allow transforming candidates, revise consult--grep

[minad]

• [x] The group function should be allowed to transform candidates in order to allow removing common prefixes.
  • [x] selectrum
  • [x] icomplete-vertical (https://github.com/oantolin/icomplete-vertical/pull/34)
  • [x] vertico
  • [ ] embark (no support yet, see https://github.com/oantolin/embark/issues/172)
• [x] Rework consult--grep to make use of this feature, see 68ec514facb0f988b29b22e0737ea4dd6c77aca7
• [ ] Reconsider the consult-grep protocol with Embark, maybe get rid of embark-consult-goto-location. This is basically a remnant from using xref-location, see also https://github.com/oantolin/embark/issues/209, moved here https://github.com/minad/consult/issues/284
• [ ] Push the specification for the group-function upstream to Emacs, xref and project search could make use of the feature (cc @dgutov)

[minad]

@dgutov On the emacs-devel mailing list I made this proposal for a group-function to be added to the completion-metadata. This function should support grouping candidates into subsets with a title and it should be possible to transform each candidate (stripping a prefix for example). Would this functionality be helpful for xref and the project search? I think there you also find some common prefixes and remove them? In the context of consult-grep I consider to use grouped completion candidates, show the file as group title and show only the line numbers in front of the matches.

[dgutov]

Would this functionality be helpful for xref and the project search?

I... don't think so.

Perhaps the (non-default) xref-show-definitions-completing-read could use it, but what behavior would it enable?

Make the user select the group first, and then the element inside? Not sure we want that.

[minad]

@dgutov Filtering by name is still possible, the name is only hidden by the transformer for display purposes in the completion UI (completions buffer, selectrum, vertico, ...). There is no need to select a group first. And yes, it would only apply to the non-default xref-show-definitions-completing-read.

[minad]

Basically the idea is to have a completions buffer which looks more like an occur buffer with titles. Does this explain it?

[dgutov]

Hm, okay. So both groups and completions will be visible, just in a different arrangement?

That sounds good, but I'm not so sure about the integration strategy. Like, we'd still need to keep the file names in the completion strings for other UIs (default one, and Ivy, Helm...).

The "transformer" thingy might remove the prefix, but then it's followed by colon, which a bit of presentation that's helpful for displaying the string in "flat" presentation, but might look odd when you strip the file name. And in other cases the group might not be the prefix of each completion.

Anyway, why not give it a try. I support the idea of an Occur-like buffer for match results, obviously.

[minad]

Hm, okay. So both groups and completions will be visible, just in a different arrangement?

Yes.

That sounds good, but I'm not so sure about the integration strategy. Like, we'd still need to keep the file names in the completion strings for other UIs (default one, and Ivy, Helm...).

Everything will be preserved if the group-function is not used. It is fully backwards compatible.

The "transformer" thingy might remove the prefix, but then it's followed by colon, which a bit of presentation that's helpful for displaying the string in "flat" presentation, but might look odd when you strip the file name. And in other cases the group might not be the prefix of each completion.

The colon is also removed by the group function. The group function does both group and candidate transformation, but it's definition is still reasonably simple.

1. If the group function is called with a list of strings (candidates), it should return a alist of groups ((group-title . group-cands)...)
2. If the group function is called with a string (a candidate), it should return a pair of transformed candidate and title.

For ordering the UI will call 1. and for transforming it will call 2. You can look at the Vertico sources for x-group-function.

I just pushed the corresponding commit to Consult and Vertico. Here is a screenshot of the Vertico UI and consult-ripgrep. The *Completions* buffer could be adjusted accordingly.

[minad]

If the UI does not support group-function or you disable it (in consult this can be done on a per-command basis) you see this:

[dgutov]

Everything will be preserved if the group-function is not used. It is fully backwards compatible.

Very good.

The colon is also removed by the group function. The group function does both group and candidate transformation, but it's definition is still reasonably simple.

So it's basically two functions ;-)

I think you can reimplement 1. in terms of 2.

Here is a screenshot of the Vertico UI and consult-ripgrep.

Looks okay. The group text is pretty thin, though.

[minad]

I think you can reimplement 1. in terms of 2.

No, you cannot. The group function also determines the order of the groups. But if you don't care about that, you are actually right. Then having a separate 1. is only an optimization.

Looks okay. The group text is pretty thin, though.

Yes, this is only my style (it is a local customization in my init.el) since this is Vertico in the minibuffer and I wanted to save space. Not the best idea probably. You may prefer larger titles like helm or titles in the same face as the candidates. Using the same face is the default in the modus themes.

[minad]

I close this here, since it is done in Consult/Vertico/Selectrum/Icomplete-vertical. @dgutov I may send some patches regarding this feature to emacs-devel at some point.

[dgutov]

No, you cannot. The group function also determines the order of the groups. But if you don't care about that, you are actually right. Then having a separate 1. is only an optimization.

Perhaps extract that logic into group-sort-function, or the like? With default sort obtained from having the completions sorted, and then grouping them by group-function.

Yes, this is only my style (it is a local customization in my init.el) since this is Vertico in the minibuffer and I wanted to save space. Not the best idea probably. You may prefer larger titles like helm or titles in the same face as the candidates. Using the same face is the default in the modus themes.

I may send some patches regarding this feature to emacs-devel at some point.

Very good. Thanks!

[minad]

Perhaps extract that logic into group-sort-function, or the like? With default sort obtained from having the completions sorted, and then grouping them by group-function.

The idea is to always execute first the display-sort-function and then the group-function to get the ordering. The group function must preserve the sorting of the candidates, it is only allowed to split the candidates into groups.

We could split the group-function in two, group-sort-function and group-transform-function but I would rather not propose that. We already have multiple functions display-sort/cycle-sort/annotation/affixation and so on, so I think it would be better to not add two functions which implement parts of a single feature. Note that affixation-function also supports different return types at least, therefore I think it is okay to have a group-function which takes two different arguments (string or list).

Furthermore I find it quite practical to use a single function. For example, the user may not want to use the transformation behavior of the group-function and only group the candidates by some title. This can then be achieved by writing such a helper function:

(defun consult--group-by-title (title candidates)
  "Group CANDIDATES according to TITLE function."
  (if (stringp candidates)
      ;; Return untransformed candidate and title
      (cons candidates (funcall title candidates))
    ;; Return groups
    (let ((groups))
      (dolist (cand candidates)
        (let* ((title (funcall title cand))
               (group (assoc title groups)))
          (if group
              (setcdr group (cons cand (cdr group)))
            (push (list title cand) groups))))
      (mapc (lambda (x) (setcdr x (nreverse (cdr x)))) (nreverse groups)))))

Another alternative one may want to consider is to add a general transform-function (which returns the transformed candidate) and a group-function (which only accepts lists and returns the grouped lists). Now we already have the affixation-function, which may also transform the candidate (is this correct?). We could simply use the affixation-function and the group-function. But there is a downside of doing that. A completing-read user may want to specify both an affixation and a group function, but the candidates should only be transformed when grouping is actually used by the UI. Therefore I am quite convinced that stuffing the feature into one function is the better approach. Keeping the different features separate and not entangling them.

[dgutov]

It not my decision really, but I don't think it's a problem, as long as the desire for a special sorting order is a rare occurrence. I haven't see display-sort-function used much (*), and if you don't use it, you'll only have group-function in the common case. And its implementation will be simpler and easier to read. I won't need the helper function at all, the frontend will take care of that.

I guess I also don't like multi-shape functions (or what should I call that?). They're okay if the corresponding API has a certain tradition, but the *-function thingies in c-a-p-f don't have such examples yet.

Depending on whether you want to name the property :consult-group-function or just :group-function, I guess you'll need to pass the review in the core, but that will be up to Stefan, I guess.

Now we already have the affixation-function, which may also transform the candidate (is this correct?)

Kind of. I haven't really tried out that feature yet. Not sure if Company can support it, for example.

A completing-read user may want to specify both an affixation and a group function, but the candidates should only be transformed when grouping is actually used by the UI.

Wouldn't that also be the case with separate group-function and group-sort-function?

[dgutov]

(*) Sorting doesn't really feel like the property of completion table. Of matching mechanism - perhaps, or it can depend on history, or on buffer contents surrounding input. But having sort function depend on the string contents (and not sort them alphabetically) would be weird.

[minad]

It not my decision really, but I don't think it's a problem, as long as the desire for a special sorting order is a rare occurrence. I haven't see display-sort-function used much (*), and if you don't use it, you'll only have group-function in the common case. And its implementation will be simpler and easier to read. I won't need the helper function at all, the frontend will take care of that.

It matters for example with the flex completion style which performs its own sorting. I am using display-sort-function and cycle-sort-function mostly to disable sorting in consult by setting it to identity. But even if the default sorting is used, the grouping should still happen after the default sorting.

Take a look at the Vertico candidate code where sorting happens:

https://github.com/minad/vertico/blob/b1d513d32fffe6f7251a457c39a1c3c5a7ef6a26/vertico.el#L266-L292

I guess I also don't like multi-shape functions (or what should I call that?).

They are just normal functions. Nothing special about them, except that they take a sum type as argument.

They're okay if the corresponding API has a certain tradition, but the *-function thingies in c-a-p-f don't have such examples yet.

You are right about that. But I am looking at it from the perspective of completing-read, you are looking at it from the perspective of capf. The proposal would also work if you add :group-function to completion-extra-properties. But for completing-read you would want to have it as part of the metadata, like the display-sort-function. And note that the group function is much more tied to the completion table than the sorting. You may just want to sort by length or alphabet, but the candidate transformation of the group function must actually know about the structure of the candidates!

Depending on whether you want to name the property :consult-group-function or just :group-function, I guess you'll need to pass the review in the core, but that will be up to Stefan, I guess.

Sure. I intend to call it :group-function since I propose it as a generally useful extension. And maybe I can provide a patch for your xref completing read which also makes use of it?

Kind of. I haven't really tried out that feature yet. Not sure if Company can support it, for example.

It should be no problem, just like the annotation-function. The candidate transformation is a bit more problematic maybe? In Corfu I use either the affixation-function or fall back to the annotation-function if specified.

https://github.com/minad/corfu/blob/0f863ef9bde64042a7cde65e0f3cb0056d095505/corfu.el#L356-L364

Wouldn't that also be the case with separate group-function and group-sort-function?

Sure, my argument here was rather targeted at using the affixation-function for transformation and the group function only for grouping. But I really don't see advantages of having these functions separate. I tested the current design with Vertico and Consult, doing a split would not make things better. I would need additional handling if only one of the functions is present and the completing-read user specifying the function would not win much. The code complexity on both sides of the completing-read API would increases. Why do you prefer to have two functions? I don't see the point.

(*) Sorting doesn't really feel like the property of completion table. Of matching mechanism - perhaps, or it can depend on history, or on buffer contents surrounding input. But having sort function depend on the string contents (and not sort them alphabetically) would be weird.

I am following the design of the display-sort-function here. And in this case the group function depends critically on the completion table content since it must know the structure of the candidates to group and transform the candidates. It is not just sorting. So it should be part of the completion table.

[dgutov]

It matters for example with the flex completion style which performs its own sorting. I am using display-sort-function and cycle-sort-function mostly to disable sorting in consult by setting it to identity.

So... you're using it to disable flex sorting? I'm not sure it's conceptually right for a completion table sort to override fuzzy matching sort, but oh well.

I've also argued in the past that completion style sorting logic should be separately configurable by the user. After all, not every user has access to the implementation of the completion table. Most don't.

But even if the default sorting is used, the grouping should still happen after the default sorting.

Not sure how my suggestion would prevent that.

They are just normal functions. Nothing special about them, except that they take a sum type as argument.

A function that takes two very different types as arguments and performs two distinctly different behaviors for them, returning two different shapes of values, very much looks like two different functions squished into one.

We do have a number of those in company-mode, and apparently you do in consult as well (IIRC sources being one example). But c-a-p-f API generally looks different. With sole exception of completion tables themselves, which are generally considered to be begging for a rewrite using cl-generic.

The proposal would also work if you add :group-function to completion-extra-properties. But for completing-read you would want to have it as part of the metadata, like the display-sort-function.

The existing distinction between metadata and extra-properties is also unfortunate.

And note that the group function is much more tied to the completion table than the sorting. You may just want to sort by length or alphabet, but the candidate transformation of the group function must actually know about the structure of the candidates!

Isn't that a good argument for group-function and group-sort-function being separate? Or for the latter never existing, and just being user-configurable on some other level.

Sure. I intend to call it :group-function since I propose it as a generally useful extension. And maybe I can provide a patch for your xref completing read which also makes use of it?

Works for me.

It should be no problem, just like the annotation-function.

IIRC affixation-function was added to show some icons for a particular input method. That won't work in company (breaking the popup layout). We have recently added a different support for icons, in a more declarative way.

Sure, my argument here was rather targeted at using the affixation-function for transformation and the group function only for grouping. But I really don't see advantages of having these functions separate.

Smaller, easier to reason implementations. Especially in the case when no sorting logic is needed, which I imagine to be the prevalent case.

I would need additional handling if only one of the functions is present and the completing-read user specifying the function would not win much. The code complexity on both sides of the completing-read API would increases. Why do you prefer to have two functions?

Code complexity would increase a little in completing-read implementations, but decrease in completion tables. I think it's safe to assume that there's going to be more completion tables than completing-read implementations.

[minad]

So... you're using it to disable flex sorting? I'm not sure it's conceptually right for a completion table sort to override fuzzy matching sort, but oh well.

No I am not disabling the sorting, I only put additional grouping on top. The flex candidates of the first group come first, etc.

A function that takes two very different types as arguments and performs two distinctly different behaviors for them, returning two different shapes of values, very much looks like two different functions squished into one.

Sure, this applies to every function taking a sum-type argument. I don't think this is an argument at all.

Isn't that a good argument for group-function and group-sort-function being separate? Or for the latter never existing, and just being user-configurable on some other level.

This is not a good idea. As I argued, the completion table must supply both functions. I don't want to have this separation and I don't want to have the functions separately configurable.

IIRC affixation-function was added to show some icons for a particular input method. That won't work in company (breaking the popup layout). We have recently added a different support for icons, in a more declarative way.

Why? You could use the icons returned by the affixation prefix and show them as prefix in company.

Smaller, easier to reason implementations. Especially in the case when no sorting logic is needed, which I imagine to be the prevalent case.

I disagree. The sorting logic is needed. I think the problem here is that you insist on having sorting separate. This is not the case and not what I am proposing. The grouping is both ordering and adding titles.

Code complexity would increase a little in completing-read implementations, but decrease in completion tables. I think it's safe to assume that there's going to be more completion tables than completing-read implementations.

No, it is won't be the case. How can you make this argument without any data to rely on? I have implementations you can look at. If you introduce two functions the code will not get simpler. On neither the side of the UI nor on the side of the table. But you can give it a try and check.

[minad]

We do have a number of those in company-mode, and apparently you do in consult as well (IIRC sources being one example). But c-a-p-f API generally looks different. With sole exception of completion tables themselves, which are generally considered to be begging for a rewrite using cl-generic.

I think you are looking at it very much from the capf pov. I had not even intended to make this proposal for capf. If you take the perspective of completing-read, then I think the proposal (and current implemention in Vertico/Consult) is reasonable. If you look at it from the capf pov you may want to have separate functions and you would want to specify those via the extra properties since this fits the capf design better.

However as you say the capf design is ugly, having both metadata and extra properties. It should be rewritten in some way or the other. But I want to take small steps here, proposing a small addition to the completing-read metadata which can help with a slightly improved UI experience while staying fully backward compatible.

[minad]

I think I am not totally opposed to splitting this in two functions group-sort-function and group-transform-function. One could implement it such that only the group-sort-function must be supplied always and the other function can be supplied on top if transformation is desired. In the case where no transformation is needed the completion table will get marginally simpler (you save two or three lines of code):

https://github.com/minad/consult/blob/44a1871deabef7c4b79ed095fc77888058bcf3ff/consult.el#L1619-L1621

But then Stefan Monnier also stated that he is not happy with having these many functions. And the UI implementation gets more complicated. Then you won't win any flexibility by doing the split. You would only win flexibility if you can disable sorting separately as you argue. But there is no possibility for that and it would not be useful to disable the sorting. I understand that you dislike squishing the functions together but that's a minor ugliness in my opinion given there are no advantages in having them separate.

[dgutov]

Sure, this applies to every function taking a sum-type argument. I don't think this is an argument at all.

Returning the value of the same shape is almost like a rule for such functions.

Why? You could use the icons returned by the affixation prefix and show them as prefix in company.

Icons need particular formatting, which is frontend-dependent.

I think you are looking at it very much from the capf pov. I had not even intended to make this proposal for capf.

But they share the underlying mechanisms.

No, it is won't be the case. How can you make this argument without any data to rely on? I have implementations you can look at. If you introduce two functions the code will not get simpler. On neither the side of the UI nor on the side of the table. But you can give it a try and check.

AFAICT, it's

(defun xrefs-groups (arg)
  (if (listp arg)
      (sort
       (seq-group-by
        (lambda (item)
          (string-match ":" item)
          (substring 0 (match-beginning 0) item))
        arg)
       (lambda (gr1 gr2)
         ;; However you want them to be sorted.
         ))
    (string-match-p ":" item)
    (cons
     (substring item 0 (match-beginning 0))
     (substring item (match-end 0)))))

vs

(defun xrefs-group (item)
  (string-match-p ":" item)
  (cons
   (substring item 0 (match-beginning 0))
   (substring item (match-end 0))))

(defun xref-groups-sort (groups)
  (sort
   groups
   (lambda (gr1 gr2)
     ;; However you want them to be sorted.
     )))

Which comes out to the same number of lines even if group sorting is needed (which I still believe is not the usual case; i.e., we definitely don't additionally sort xref groups).

The first function is more complex, and it contains duplication of the completion->group logic.

[dgutov]

One could implement it such that only the group-sort-function must be supplied always and the other function can be supplied on top if transformation is desired.

Exactly.

But then Stefan Monnier also stated that he is not happy with having these many functions.

If @monnier won't buy my arguments here, there's not much else I can do.

[minad]

AFAICT, it's ... vs ...

This is not simpler. It is exactly as simple as before.

The first function is more complex, and it contains duplication of the completion->group logic.

What kind of duplication? You mean the single conditional? This is all you save.

Which comes out to the same number of lines even if group sorting is needed (which I still believe is not the usual case; i.e., we definitely don't additionally sort xref groups).

There is no downside in additionally sorting the groups. You could kick that out any preprocessing from your table which ensures that the candidates are grouped before hand. Furthermore the grouping happens in linear time not in O(nlogn). You never want to disable the grouping. In particular given the flex style which does its own sorting, you still want to preserve the grouping otherwise you will end up with an ui showing this, where the groups are split up:

group 1
cand
group 2
cand
group 1
cand
group 2
cand
...

If @monnier won't buy my arguments here, there's not much else I can do.

Yes, but I am not really in the mood to propose this if I cannot convince you here. Does the argument sound reasonable to you, that you don't want to disable the grouping separately? This is a key point of the proposal. Unfortunately as you stated here you don't buy this argument:

(which I still believe is not the usual case; i.e., we definitely don't additionally sort xref groups).

[minad]

Maybe my idea is too much tied to a UI like Vertico/Selectrum/Helm in contrast to the usual Completions buffer. This is the reason why I insist on not having the ability to disable the grouping separately from the transformation/titles. (The UI could still offer an option to disable grouping if that's desired, but then you disable both grouping and titles as in the screenshot above).

[dgutov]

This is not simpler. It is exactly as simple as before.

In the usual case it will be 5 lines vs 15.

And here you have an extra conditional, weird argument naming, and duplicated call to the function which returns the group name in both branches.

You could kick that out any preprocessing from your table which ensures that the candidates are grouped before hand.

I don't need extra sorting in the xref's case at least. But I never said I want to disable grouping.

In particular given the flex style which does its own sorting, you still want to preserve the grouping otherwise you will end up with an ui showing this, where the groups are split up:

I... don't think so?

I mean, now that I see this example I get that you want to have the possibility to group while strictly keeping the ordering of completions (instead of only having one group for each group name, like seq-group-by will do). I'm just not sure what exact use cases will be served by this.

Yes, but I am not really in the mood to propose this if I cannot convince you here.

Recent events show my opinion doesn't have too much weight, so don't let that stop you.

[minad]

And here you have an extra conditional, weird argument naming

Agree.

and duplicated call to the function which returns the group name in both branches.

Disagree. This is an optimization and added functionality in the first branch since it allows transformation.

I don't need extra sorting in the xref's case at least. But I never said I want to disable grouping.

So which function would your xref table supply then? In the case with a single function you would have to supply this single function and in the case of two functions you would have to supply both functions, the one which returns the groups and the one which returns the titles/transforms. Alternatively you could only supply the function which transforms but then you would get the split up grouping with the flex style. And this use case is not something I had intended in my proposal.

I mean, now that I see this example I get that you want to have the possibility to group while strictly keeping the ordering of completions (instead of only having one group for each group name, like seq-group-by will do). I'm just not sure what exact use cases will be served by this.

No, I want to always have one group for each group. But inside the groups the original sorting determines the order. So I am explicitly against keeping the full original sorting of the candidates to prevent the ui from looking as I shown above.

[dgutov]

This is an optimization and added functionality in the first branch since it allows transformation.

For what purpose? Only one version of group name is ever shown.

So which function would your xref table supply then?

The first one.

Alternatively you could only supply the function which transforms but then you would get the split up grouping with the flex style.

Why? Take the return value of seq-group-by, it doesn't split groups.

[minad]

For what purpose? Only one version of group name is ever shown. The first one.

For the purpose of removing a prefix from the candidates for example as in the screenshots shown above. Having the transformation also useful for xref in order to achieve a UI which looks more like occur. If you only supply the first function you cannot get the prefix removal.

[dgutov]

But xrefs-group in the example above does implement prefix removal as well, doesn't it?

[minad]

Ah I see, it is not intended like this and not according to the specification I had in mind. I don't think you cannot make it work like this from the side of the completion UI. The grouping must preserve the original candidates.

[minad]

If the grouping does not preserve the original candidates we cannot use the group function to perform the ordering after the sorting. It is crucial to preserve the candidates as is in this processing step. Only right before displaying we can transform the candidates.

[dgutov]

The original candidates can still be preserved by the completing-read implementation itself. It will just build associations with group names and "truncated" versions based on what group-function returns.

[dgutov]

Worst case: you can call group-function twice (once for grouping, one before display). Maybe even (okay, function squishing alert) introduce an argument which will determine whether the function should return the group name or the truncated candidate name, for best performance.

[minad]

The original candidates can still be preserved by the completing-read implementation itself. It will just build associations with group names and "truncated" versions based on what group-function returns.

No, this will be inefficient. My proposal is made specifically such that it can be implemented efficiently. I don't think the discussion is useful anymore at this point. I suggest you give consult+vertico a try and then patch it such that the result fits you better. Then we can discuss this further. But the counter proposal you are making here is not feasible.

[minad]

Worst case: you can call group-function twice (once for grouping, one before display).

I am already doing that. Therefore the two functions squished in one.

Maybe even (okay, function squishing alert) introduce an argument which will determine whether the function should return the group name or the truncated candidate name, for best performance.

Okay, but you accept now that we need the two functions?

[dgutov]

Okay, but you accept now that we need the two functions?

We're basically talking about three different functions by this point. Squishing two of them together seems okay-ish to me (because in those cases at least the input type/value is the same).

[dgutov]

And it will look like this:

(defun xrefs-group (item truncation?)
  (string-match-p ":" item)
  (if truncation?
      (substring item (match-end 0))
    (substring item 0 (match-beginning 0))))

No weird argument naming, no dispatch on argument type, no code duplication.

[minad]

Okay, so you want to have only this one function which returns titles and truncated candidates? But then the group function has no final say about the ordering (I mentioned that very much in the beginning when you proposed to implement the two functions as one). You may want to always preserve the order such that the first group is always first even if flex reorders the candidates.

[dgutov]

I'm not sure I understand. What is "first group" if not the group of the first candidate (after sorting, let's say)?

But then the group function has no final say about the ordering (I mentioned that very much in the beginning when you proposed to implement the two functions as one).

It won't be able to re-sort candidates inside groups, that's true. Should it be able to?

[minad]

What is "first group" if not the group of the first candidate (after sorting, let's say)?

The first group is the first group as determined by the group function. Even if the sorting says otherwise the group function has the final say which group comes first. But usually the group function can just accept the order as given by the sorting function and make the group of the first candidate the first group.

The consult--group-by-title function makes the group of the first candidate the first group. See here:

https://github.com/minad/consult/blob/44a1871deabef7c4b79ed095fc77888058bcf3ff/consult.el#L1617-L1630

But this is not a necessity. I would like to preserve the flexibility of the group function to determine the group order.

[minad]

If we would not want this property of the group function, then we could just get away with a single function

(defun title-function (cand)
    ...
   (cons transformed-cand title))

This would be the simplest proposal. The completion system calls this function on each candidate to achieve the grouping and for display purposes and so on.

[minad]

Now given that we can also supply a sorting function one could get away with the title-function proposal. But then I don't want to always supply a sorting function, it should also work with the default sorting function as used by the completion system and also with flex sorting. In those case it is necessary to allow the group function to have the final say over the group order. Otherwise you would get jumping groups in the case of flex for example, you enter something, then group1 is first, you enter more, then suddenly group2 is first. I don't think that is really desired for something like a consult-buffer command.

[monnier]

(*) Sorting doesn't really feel like the property of completion table. Of matching mechanism - perhaps, or it can depend on history, or on buffer contents surrounding input.

Indeed, if you search for display-sort-function you'll see it's used quite rarely (tho these rare uses do make some sense: it's typically because the strings have a "natural" ordering that comes from something else than the string itself, such as the char-code for unicode char names or, as you mention, some notion of recency/history as is the case in ecomplete or in completion of kill-ring items).

[minad]

@monnier @dgutov I agree with sorting not being a property of the completion table. I am usually just relying on the default sorting function as used by the completion ui. However the group function I am proposing is different. The group function is tied to the table and must have knowledge of the structure of the candidates.

[monnier]

I guess I also don't like multi-shape functions (or what should I call that?).

FWIW, I don't much like them either. I was taught that a function which looks like

(defun foo (arg1 other ... args))
  (cl-case arg1
    [the rest only uses other..args]))

and where the actual arg1 args are usually constants is often a sign of bad style and it would be better replaced by N function.

[dgutov]

The first group is the first group as determined by the group function.

Based on what principle, though?

It's either the group of the first completion before sorting, or the group of the first completion after sorting. But given that the first completion can move arbitrarily down the list during sorting, and we don't want to have split groups, it should be the group of the first completion after sorting, right?

In those case it is necessary to allow the group function to have the final say over the group order. Otherwise you would get jumping groups in the case of flex for example, you enter something, then group1 is first, you enter more, then suddenly group2 is first.

I can see how that could be a problem. But then you just take the groups and sort them alphabetically or whatever (not sure how the xref completion table would make that choice for you).

But I can easily imagine the user prefer to have the groups "jumping" after keypresses when the ordering of completions changes.

After all, if the completion style matches both the group name and the string inside, me typing a new char can mean that another group (file name, perhaps) matches my input better. Why shouldn't it be at the top now?

[monnier]

But then Stefan Monnier also stated that he is not happy with having these many functions.

I must say I can't remember what this refers to (neither in the sense of what I said, nor in the sense of which functions we're talking about).

I'm curious about the relationship with the design you propose and the end result you need. I think I don't quite understand it all yet.

AFAICT what you need in the end is a list of groups where every group has some description and a list of elements, and every one of those elements is basically a pair of "the raw string" and "the presentation string" where the presentation string may have been streamlined according to the grouping.

So an obvious "single function" solution is a group-function which returns an alist where each entry's key is a group name and each value is a list of pairs.

I'm pretty sure you thought of that as well, but what I don't know is what are the reasons why you went with a different design. Could you clarify?

[minad]

@dgutov

Based on what principle, though?

Based on whatever principle the author of the completion table decides on. For certain completion tables (in particular of multi sources) it may make sense to always show the groups in a predefined order. But maybe this is really not that important and we could just get away with the simpler title-function. When I originally designed this I wanted to have the flexibility of the group function.

But I can easily imagine the user prefer to have the groups "jumping" after keypresses when the ordering of completions changes.

Yes, I can also imagine that. But I can also imagine the contrary. For example for consult-buffer I always want the buffers first since these are the "fast access" candidates, while the candidates coming in the later groups are like auxiliary candidates you may access less frequently. For example if consult-buffer combines recent files and buffers, the recent files can always come later, since after you've opened a recent file it is promoted to a buffer.

After all, if the completion style matches both the group name and the string inside, me typing a new char can mean that another group (file name, perhaps) matches my input better. Why shouldn't it be at the top now?

Agree.

---

I think given my current experience I would like to have an ordering determined by the group function for multi-source commands (virtual buffers) but not in the case of your xref-completing-read. There I would be more happy with jumping. However in my Consult code base I already disable sorting for consult-buffer by setting the sorting functions to identity. This won't work for the flex completion style, which is acceptable. Furthermore I also disable sorting for consult-ripgrep which is like your xref-competing-read. Given this my argument to have the group function determine the group order is mood, I would be happy with only a title function I guess.

[monnier]

@monnier @dgutov I agree with sorting not being a property of the completion table. I am usually just relying on the default sorting function as used by the completion ui.

Yes, indeed, my message was just pointing out that "everyone" agrees that completion-tables almost never want to nor need to care about sorting.

However the group function I am proposing is different.

Fully agreed as well.

The group function is tied to the table and must have knowledge of the structure of the candidates.

Indeed.

[minad]

@monnier

So an obvious "single function" solution is a group-function which returns an alist where each entry's key is a group name and each value is a list of pairs.

Yes, I have indeed considered it. And I didn't use it in order to improve the performance. I always want to group the candidates, but the transformation is only necessary for the display.

[minad]

@dgutov @monnier But given the discussion, I think having a title-function which returns a pair of transformed candidate and group title is all one needs. The performance difference will be negligible given that the transformation is always simple, since one usually only removes some information from the candidate which is now displayed in the group title.

Then the UI can call this function on each candidate to group the candidates and also to obtain the titles and transform the candidates for display.

Alternatively one may go with the group function as described by @monnier (alist of groups, where each group element is a pair of candidate and transformed candidate). This pushes a bit of the complexity from the UI to the completion table without clear benefits, but would be feasible as well. You can go back and forth between these different representations.

What do you think?