[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#43598: replace-in-string: finishing touches
From: |
Lars Ingebrigtsen |
Subject: |
bug#43598: replace-in-string: finishing touches |
Date: |
Fri, 25 Sep 2020 13:11:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Mattias Engdegård <mattiase@acm.org> writes:
> 1. Check the range of the START-POS argument so that we don't crash.
> The permitted range is [0..N] where N is (length HAYSTACK), thus we
> permit a start right after the last character but no further.
> We could also return nil in these cases but I think an error is more useful.
Good point. :-)
> 2. Make the docs more precise about various things.
>
> 3. Slight simplification of the implementation logic to avoid testing
> the same conditions multiple times.
>
> 4. More tests, especially for edge cases. Can't have too many!
It all looks good to me; please apply.
> One test still fails:
>
> (string-search "ø" "\303\270")
>
> which should return nil but currently matches.
> I think it's wrong to convert the needle to unibyte (using
> Fstring_as_unibyte) in this case, but I haven't decided what the best
> solution would be.
Yeah, that's the bit I was most unsure about, because it just didn't
look quite correct to me, but I couldn't come up with the correct test
case last night; thanks.
> We should also consider the optimisations:
> - If SCHARS(needle)>SCHARS(haystack) then no match is possible.
Yup.
> - If either needle or haystack is all-ASCII (all bytes in 0..127),
> then we can use memmem without conversion.
Right, so if the multibyteness differs, then do another check to see
whether both strings are all-ASCII anyway, and do the comparison without
conversion... Yes, makes sense to me.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
- bug#43598: replace-in-string: finishing touches, Mattias Engdegård, 2020/09/24
- bug#43598: replace-in-string: finishing touches, Lars Ingebrigtsen, 2020/09/24
- bug#43598: replace-in-string: finishing touches, Mattias Engdegård, 2020/09/25
- bug#43598: replace-in-string: finishing touches,
Lars Ingebrigtsen <=
- bug#43598: replace-in-string: finishing touches, Mattias Engdegård, 2020/09/25
- bug#43598: replace-in-string: finishing touches, Lars Ingebrigtsen, 2020/09/25
- bug#43598: replace-in-string: finishing touches, Lars Ingebrigtsen, 2020/09/26
- bug#43598: replace-in-string: finishing touches, Lars Ingebrigtsen, 2020/09/26
- bug#43598: replace-in-string: finishing touches, Mattias Engdegård, 2020/09/27
- bug#43598: replace-in-string: finishing touches, Richard Stallman, 2020/09/27
- bug#43598: replace-in-string: finishing touches, Mattias Engdegård, 2020/09/28
- bug#43598: replace-in-string: finishing touches, Richard Stallman, 2020/09/28
- bug#43598: replace-in-string: finishing touches, Eli Zaretskii, 2020/09/29
- bug#43598: replace-in-string: finishing touches, Mattias Engdegård, 2020/09/27