[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses sa
From: |
João Távora |
Subject: |
bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator |
Date: |
Sat, 25 Nov 2023 00:03:10 +0000 |
On Fri, Nov 24, 2023 at 9:51 PM Jonas Bernoulli <jonas@bernoul.li> wrote:
> I haven't found any issues beside this off-by-one font-lock issue.
Good.
> So far I have used this beauty:
>
> diff --git a/lisp/emacs-lisp/shorthands.el b/lisp/emacs-lisp/shorthands.el
> @@ -57,7 +57,7 @@ shorthands--mismatch-from-end
> for i from 1
> for i1 = (- l1 i) for i2 = (- l2 i)
> while (and (>= i1 0) (>= i2 0) (eq (aref str1 i1) (aref str2 i2)))
> - finally (return (1- i))))
> + finally (return (if (eq (aref str2 (1+ i2)) ?-) (1- i) i))))
>
> Is that good enough? Depending on how you look at it, this changes what
> is being returned, but IMO this function is a bit murky to begin with.
A bit dodgy, no :-) Maybe a docstring would shed some light
on what this function is supposed to do, so I'll propose one below.
> The function name is `shorthands--mismatch-from-end', but it returns the
> length of the common suffix, minus one, to account for the separator.
> This change ensures that the separator is accounted for, even if it
> differs between the shorthand and real symbol.
>
> Since this function returns the length of the *matching* suffix after
> the prefixes (including separator), I find it weird that its name
> contains *MISmatch*.
Probably I wanted to emulate what CL's MISMATCH does to some degree.
cl-mismatch exists in Emacs, but it's not available in shorthands.el,
and it seems to be buggy anyway.
You can rename the function shorthands--common-suffix-length if
you want. I probably meant it to be separator-agnostic function,
and be replaceable by a cl-mismatch some time around 2084,
Now to your problem: I think what you want is to customize element
comparison (CL's MISMATCH allows that, btw). Here's one way.
diff --git a/lisp/emacs-lisp/shorthands.el b/lisp/emacs-lisp/shorthands.el
index 82200ab88e9..36a862bc037 100644
--- a/lisp/emacs-lisp/shorthands.el
+++ b/lisp/emacs-lisp/shorthands.el
@@ -52,11 +52,18 @@ elisp-shorthand-font-lock-face
:version "28.1"
:group 'font-lock-faces)
-(defun shorthands--mismatch-from-end (str1 str2)
- (cl-loop with l1 = (length str1) with l2 = (length str2)
+(defun shorthands--mismatch-from-end (str1 str2 &optional test)
+ "Compute position of first mismatching element of STR1 and STR2, from end.
+The return value is the reverse index of that element. If 0, the
+last characters of STR1 and STR2 differ, if 1, the penultimate
+characters differ, and so on. If optional TEST is passed,
+compare elements using that function, else compare with `eq'."
+ (cl-loop with test = (or test #'eq)
+ with l1 = (length str1) with l2 = (length str2)
for i from 1
for i1 = (- l1 i) for i2 = (- l2 i)
- while (and (>= i1 0) (>= i2 0) (eq (aref str1 i1) (aref str2 i2)))
+ while (and (>= i1 0) (>= i2 0)
+ (funcall test (aref str1 i1) (aref str2 i2)))
finally (return (1- i))))
(defun shorthands-font-lock-shorthands (limit)
And now using the following lambda for TEST yields what you want:
(shorthands--mismatch-from-end "s-tail" "long-tail") ;; => 5
(shorthands--mismatch-from-end "s/tail" "long-tail"
(lambda (c1 c2)
(or (and (eq c1 ?/) (eq c2 ?-))
(eq c1 c2)))) ;; => also 5
Of course, you can hardcode this test inside the function, no need
for a parameter, and rename the function to whatever you find more
appropriate. This allows us to keep control over what things
are considered acceptable separators from a font-locking perspective
> It might make more sense to return the length of the shorthand prefix.
I've been away from this code for a couple of years, so feel free to
propose more extensive changes. As long as they don't increase the
complexity and are suitably tested, I have nothing against.
> Also, have you considered throwing in a
> (not (string-equal (match-string 1) sname))
>
> to avoid having to call `shorthands--mismatch-from-end' at all?
Where and how this be thrown in? How would you know what to highlight
in the shorthand printed form? There can be many shorthand prefixes
in a given file. But do show some code.
> Maybe you have, but concluded it is cheaper to do a bit too much work
> for non-shorthands, than to effectively repeat some work for shorthands.
Maybe. Not sure this is more work (string-equal must still compare
every character right?), but, in summary, I'm not married to this
implementation. I somewhat appreciate that I could still read it
after not having looked at it for a couple years, but feel free to
change it.
João
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Jonas Bernoulli, 2023/11/22
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, João Távora, 2023/11/23
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Jonas Bernoulli, 2023/11/24
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator,
João Távora <=
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Joseph Turner, 2023/11/24
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Jonas Bernoulli, 2023/11/25
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Joseph Turner, 2023/11/25
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, João Távora, 2023/11/26
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Joseph Turner, 2023/11/26
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, João Távora, 2023/11/26
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Joseph Turner, 2023/11/26
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Eli Zaretskii, 2023/11/27
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, João Távora, 2023/11/29
- bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator, Joseph Turner, 2023/11/29