Match highlighting when using display property?

[bdarcus]

I got this bug report ...

emacs-citar/citar#47

... which if I understand right is noting that the candidate matches are not highlighted in my implementation, I believe because i'm using the display property for the candidates.

First, is my understanding of highlighting and the display property correct? Seems yes, as if I type "edi" here, that substring is not highlighted.

(completing-read ";) "
                 (list (propertize "vim evil mode editor" 'display "vim editor")
                       (propertize "emacs best editor" 'display "emacs editor")))

Is there a reason this shouldn't be supported (logically I can see an argument why it shouldn't be)? I note in ivy it is, just for comparison.

Edit: there may be an additional issue the poster was noting with highlighting with this scenario, but I'm not sure.

[minad]

Highlighting is not supported and should not be supported for the display strings. This is how completion styles work. They only highlight the actual string. If bibtex-actions overwrites the whole candidates with 'display strings, its a bug in bibtex-actions.

[bdarcus]

If bibtex-actions overwrites the whole candidates with 'display strings, its a bug in bibtex-actions.

Makes sense, but Is it even a bug at all though?

I.mean I need to use the separate display string, so I have no other alternative right?

[minad]

I generally don't overwrite the part of the candidate with display properties, where I want to match on. Display properties can only be used to add annotation-like prefixes, suffixes or to hide/overwrite some unique unicode prefixes as I am doing in consult-line with the tofus.

[bdarcus]

And just so I am clear, the only thing I lose with my approach is the substring highlighting? Or is there something else? Because I'm relying on strings created in my dependent package, and doing anything else would require me to add a few hundred lines of code to replace that. If that's the case, doesn't seem worth the hassle, and it's not a bug; just a trade-off. Right?

[minad]

I personally would not be happy with the trade-off. I would want to see the matching highlights. Is there not a possibility to normalize the strings somehow, such that they are well-formed, not unnecessarily using display properties etc? Instead of rewriting the whole formatter, you could just write a small converter (which certainly won't take hundreds of lines). Furthermore it may make sense to try again to merge your work into the main bibtex-completion package. Then you can also adjust the formatter there such that it produces the strings in the form you prefer, if that is possible such that ivy-bibtex/helm-bibtex continue to work.

[minad]

Or is there something else?

One thing I want to emphasize - the completion style does not match on the display properties and in consequence it does not highlight those strings. All properties are just ignored, so for the whole matching all that matters is really the string. Therefore I don't think replacing everything with 'display is generally a good idea.

[bdarcus]

I personally would not be happy with the trade-off. I would want to see the matching highlights.

OK.

Is there not a possibility to normalize the strings somehow, such that they are well-formed, not unnecessarily using display properties etc? Instead of rewriting the whole formatter, you could just write a small converter (which certainly won't take hundreds of lines).

The way bibtex-completion is designed, there's a variable that allows you to add additional fields to the candidates. If I add url and tags to that variable, which are not included by default in the search string (but tags, in particular, are valuable for search), here's a candidate from bibtex-completion-candidates.

 ("✎ far right,insurrection,protest,research https://www.washingtonpost.com/national-security/capitol-riot-fbi-intelligence/2021/01/12/30d12748-546b-11eb-a817-e5e7f8a406d6_story.html 2021-01-12 Barrett, Devlin and Zapotosky, Matt FBI Report Warned of 'war' at Capitol, Contradicting Claims There Was No Indication of Looming Violence - the Washington Post online barrett2021"
  ("=has-note=" . "✎")
  ("tags" . "far right,insurrection,protest,research")
  ("url" . "https://www.washingtonpost.com/national-security/capitol-riot-fbi-intelligence/2021/01/12/30d12748-546b-11eb-a817-e5e7f8a406d6_story.html")
  ("date" . "2021-01-12")
  ("author" . "Barrett, Devlin and Zapotosky, Matt")
  ("title" . "FBI Report Warned of 'war' at Capitol, Contradicting Claims There Was No Indication of Looming Violence - the Washington Post")
  ("=type=" . "online")
  ("=key=" . "barrett2021"))

So what you're suggesting here is instead of using the display-format function, instead write some small formatter (or modify bibtex-completion-format-entry to do this) that takes the cons cells there and replaces the candidate with probably:

1. what is important to see (author, title, date)
2. maybe put the rest in an invisible property?

Note on the 2, I added in my implementation ability to search also for presence of associated PDFs and notes, by looking for the "=has-note" etc cons cell and if present, appending "has:note" etc to the search string, because bibtex-completion uses a symbol to indicate that, which is good for display, but bad for search.

This is what bibtex-actions candidates look like now.

(#("✎ far right,insurrection,protest,research https://www.washingtonpost.com/national-security/capitol-riot-fbi-intelligence/2021/01/12/30d12748-546b-11eb-a817-e5e7f8a406d6_story.html 2021-01-12 Barrett, Devlin and Zapotosky, Matt FBI Report Warned of 'war' at Capitol, Contradicting Claims There Was No Indication of Looming Violence - the Washington Post online barrett2021 has:note" 0 380
    (display "Barrett, Zapotosky     FBI Report Warned of 'war' at Capitol, Contradic   2021" bibtex-actions-suffix "          barrett2021        online          far right,insurrection,protest,research                                                                                                                                "))

It works VERY well for search, and the display is both nice and configurable. But as you note, we lose match highlighting in the process.

Furthermore it may make sense to try again to merge your work into the main bibtex-completion package. Then you can also adjust the formatter there such that it produces the strings in the form you prefer, if that is possible such that ivy-bibtex/helm-bibtex continue to work.

I already submitted a couple of PRs for bibtex-completion for just this reason. One of them potentially relates to this.

tmalsburg/helm-bibtex#367

So this is for the key "display" formatting function, to allow it to take a template as optional parameter, in order to generate the suffix and annotation.

But given this discussion, I may need to rethink that.

[bdarcus]

PS - another nice feature of the display format function in bibtex-completion is it's fully user configurable. So in my implementation, both main display and sufix/annotations are user configurable. The bibtex-completion PR I note above adds the latter, but maybe this could be generalized to address this.

[bdarcus]

Actually, I think this is easier than I thought; I can use bibtex-completion-format-entry to format the candidate (so don't use bibtex-completion-candidates at all), and then a little function to generate additional metadata I put in an "invisible" property.

I think that retains the best of the current implementation while adding match highlighting on the key pieces of the candidate.

If you see any problem with this approach, let me know @minad, and thanks much!

[minad]

@bdarcus This is exactly the approach I would have taken!

[bdarcus]

@bdarcus This is exactly the approach I would have taken!

Good to know!

Can I please ask help on one last, what should be simple, thing?

I need something like this for the candidates, I think, but this itself is not working.

(propertize "ABC" 'invisible "INV" 'suffix "SUFF")
#("ABC" 0 3
  (invisible "INV" suffix "SUFF"))

So I need:

1. "ABC" to be visible, and searchable.
2. "INV" to be hidden, but also searchable.
3. "SUFF" to be there, so I can access it in the suffix/annotation function, but not visible or searchable (or maybe I shouldn't even use affixation or annotation given all this, and instead including directly here using a separate face?)

I have tried tons of variations of this, including using concat, but none work exactly as I outline.

[minad]

(concat (propertize "visible-and-searchable" 'my-tag my-data) " "
        (propertize "invisible-and-searchable" 'invisible t) " "
        (propertize " " 'display "visible-but-not-searchable"))

[bdarcus]

Just in case anyone is interested at some point, here's the commit that implements the changes discussed here.

[bdarcus]

Can I ask a followup to this @minad, in part because I saw you started a thread at emacs-devel on improving completing-read?

So ivy and helm both have a notion of a display-transformer (ivy) or :filtered-candidate-transformer (helm). These are functions that take a visible candidate subset (not the whole collection), and transform them for display.

In those system highighting works even on the transformed candidates.

So completing-read has no such concept, and the alternative is creative use of things like display or invisible properties, per here.

But two limitations of that:

1. have to create all those strings upfront, for the whole collection
2. lose highlighting if using display properties

Do I understand all that correctly?

[minad]

How does highlighting/matching work on the transformed candidates? The transformation is only applied on the already filtered candidates. This means matching is not possible and highlighting is at best useless on the transformed parts of the candidate.

[bdarcus]

@minad - I don't actually know how it works internally. I just know it works, and is useful in the context of helm/ivy-bibtex, since the display data and the searched data overlap substantially, but is formatted differently. And performance is extremely good in both cases. I think that may, along with caching, have something to do with the basic distinction between a very fast function that creates the candidates, and then a slower formatting function that transforms incrementally for display. Does that make sense?

[minad]

I just know it works, and is useful in the context of helm/ivy-bibtex,
since the display data and the searched data overlap substantially, but is
formatted differently.

Well, it is just wrong. Obviously there is no problem with performance, since it is only shown for a few candidates. It is like expensive Marginalia annotation functions.

[bdarcus]

So what's "wrong" then? The feature appears to be useful, certainly in this context, but also more generally in helm and ivy.

[minad]

So what's "wrong" then?

It is wrong to apply highlighting of matches to the transformed candidates since the matches there do not reflect the actual matches of the original string. For example if you match with some regexp, it could very well match some other parts in the transformed candidate. But these highlighted matches are then meaningless and "wrong".

[minad]

However it is not very harmful to do this and if the transformed candidate looks mostly like the original candidate you will be fine. In your bibtex-actions package I think that is the case. You add there a lot of these "author=... title=..." fields if I understood correctly, which are then hidden. But you still would like to see the matches in the displayed string. I have to think about a solution for this.

[bdarcus]

However it is not very harmful to do this and if the transformed candidate looks mostly like the original candidate you will be fine. In your bibtex-actions package I think that is the case. You add there a lot of these "author=... title=..." fields if I understood correctly, which are then hidden. But you still would like to see the matches in the displayed string. I have to think about a solution for this.

I was prompted to think about this again by this issue:

emacs-citar/citar#69

Bibtex-completion was extracted from helm-bibitex initially, I believe, so that it could provide a common backend for it and the ivy frontend.

Not surprisingly, then, it's designed around this feature.

The candidate strings it generates are not for display; only for search/filter, and include a common set of data (title, author, date, etc.), plus whatever the user wants to add in an "additional-search-fields" variable.

The display is then separately user-configurable.

In turn, bibtex-completion provides a caching implementation for bibtex-completion-candidates, so that the candidates loading is really efficient.

But for my implementation, I have to transform those candidate strings to work for completing-read, which is not very efficient, and is not cached.

I've proposed to the bibtex-completion author adding a hook to bibtex-completion-candidates so I can define a different formatting function. This seems like the ideal solution, as I get the candidates cached.

But of course, if we had a feature like this, I wouldn't have to depend on that.

[minad]

But of course, if we had a feature like this, I wouldn't have to depend on that.

You mean here or in Vertico? I don't think this will happen. Since as I say - it is wrong or to be more diplomatically can lead to misleading matches. Note that Selectrum/Vertico tries to stay closely with default completion. And there you also don't have this possibility.

The candidate strings it generates are not for display; only for search/filter, and include a common set of data (title, author, date, etc.), plus whatever the user wants to add in an "additional-search-fields" variable.

I think this is actually a hack. In the context of orderless we discussed matching on additional data. But what is actually wrong with using strings like this in bibtex-actions?

authors, title, year, publisher [abstract=... something=... something=...]

where the part in brackets is invisible. You can match on everything but you will only see matches for the first part of the string. I understand that the helm/ivy design is a bit more flexible, but I also like if the simple/minimal approach works well. This is the general spirit of the completing-read packages.

[bdarcus]

Nothing wrong with it in general, and is what I'm doing. I just lose the ability to use the cached bibtex-completion- candidates data.

[minad]

Can you add your own cache on top? But we discussed this multiple times - in the end this should just be merged into bibtex-completions then you get the needed freedom for the perfect solution, with no downsides for the ivy/helm frontends as I see it. But if the original author does not like it, you cannot do much about it. One can always consider forking. But I guess for long term maintenance is probably better to collaborate.

[bdarcus]

You summarize the options I am considering, except for forking bibtex-completion :-)

So far, I've just been adding modifications I need to my package, with intention to remove them if my change requests are accepted.

I just don't want to do too much of that. The hope I had was my package could stay very small and focused.

This may have to be another example, where I add a cache.

[bdarcus]

Maybe the cache isn't too hard after all :-)

emacs-citar/citar#71