bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator

[Joseph Turner]

João Távora <joaotavora@gmail.com> writes:

> On Sat, Nov 25, 2023 at 10:43 PM Joseph Turner <joseph@ushin.org> wrote:
>>
>> Jonas Bernoulli <jonas@bernoul.li> writes:
>>
>> > Joseph Turner <joseph@ushin.org> writes:
>> >
>> >> +                          (car (shorthands--find-if
>> >> +                                (lambda (short)
>> >> +                                  (string-prefix-p short (match-string 
>> >> 1)))
>> >> +                                read-symbol-shorthands #'car)))))
>> >
>> > Or simply:
>> >   (car (assoc (match-string 1)
>> >               read-symbol-shorthands
>> >               #'string-prefix-p))
>>
>> Much nicer - see patch.  Thanks, Jonas!
>
> So, I had a look at this patch and I think we should compare it
> with the patch after my sig, which keeps 'shorthands--mismatch-from-end'
> and also fixes this bug.
>
> The main difference I see is that my patch keeps doing one string
> comparison, via the mismatch function (which btw is now perfectly
> analogous to CL mismatch and thus correctly named).  In the worst case,
> Josheph's patch does 1 + N where N is the number of shorthands.  So
> this is a fundamental complexity change.
>
> Normally, that would be the end of the story, but here, it isn't.
> For two reasons.
>
> My version keeps a behaviour that can be considered buggy.
> If a shorthand prefix has a common suffix with the longhand prefix
> then that suffix will not be highlighted. Like:
>
> ;; Local Variables:
> ;; read-symbol-shorthands: (("bcrumb-" . "breadcrumb-")
> ;; End:
>
> Here only "b" would be highlighted, effectively showing the user
> how much typing was saved.  Is this wrong?  Does it makes sense
> to use shorthands like this?

I would expect the entire the shorthand to be highlit, I don't feeling
strongly about this.

> The other reason why this isn't the end of the story is that even
> if we take that bug for granted, the string comparison functions in
> Joshep's patch delegate to built-in C  comparison operators, which are
> often much, much faster than Elisp.  At least before the advent of native
> compilation, it used to be like this. Of course for a large enough  N
> number of shorthands, my version wins, but that is probably not very
> common either (or is it?  Not very hard to imagine a file making use
> of many libraries and shorthanding each of them?)
>
> So, benchmarking it will have to be, I'm afraid, because AFAIK
> font-locking is a very performance sensitive area of Emacs.

Yes.  I would like to learn how to do this!

> In the meantime I will push my patch, but keep the bug open to see
> if it is worth pushing Joseph's version.

Thank you!!  I'm happy to discuss this further if others are interested.

Joseph