188865 - Add a RFindCharInReadable function

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

Currently, there is RFindInReadable for searching strings in strings from right
to left, and FindCharInReadable for finding characters in strings from the left.

What's missing is RFindCharInReadable, to find the last occurence of a character
(like strrchr)

would come handy for bug 188586 comment 3

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

-> scc

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

taking

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It would be much more efficient to write this the way FindCharInReadable is
written.  (FWIW, see also bug 82054.)  You'd need to add rfind to nsCharTraits.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 133102 [details] [diff] [review]
patch

ok... I'll do that

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: implement&use nsCharTraits<...>::rfind

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 133148 [details] [diff] [review]
implement&use nsCharTraits<...>::rfind

nevermind. this hangs when given a multiple-fragment string

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: fix hang

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 133156 [details] [diff] [review]
fix hang

I created two test programs to test the chartraits changes and then the
RFindCharInReadable functions. I can attach them if you want

Comment 10

[Brendan Eich [:brendan]]

Do you get better code with the n-- loop control, where n is induced by s, or
would you get better code if you saved the initial s in t, added n-1, and looped
while s >= t?

/be

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 133156 [details] [diff] [review]
fix hang

I think it would be better to have RFindCharInReadable take the first iterator
as a const reference and the second iterator as a non-const reference, and
modify the second when the character is found.	This allows for repeated
searches using the same iterators, and would also simplify the implementation.

(Others may disagree, though.)

Other than that, it looks fine, although I didn't look at the code in
nsReadableUtils.cpp too closely.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

darin's string update probably bitrotted this a lot... don't think I'll update
this anytime soon, -> default owner