Use codepoint index for indices/1, index/1 and rindex/1

[wader]

Previsouly byte index was used.

Fixes #1430, fixes #1624, fixes #3064.

[wader]

To make this even more efficient i guess we would need to count codepoints inside memmem somehow

[wader]

Haven't entirely convinced myself yet that it should be fine to look for matches using the byte representation. Assuming both the needle and haystack is valid utf-8 i'm thinking it should be fine because of utf-8's self-synchronization property.

Update: now looking at jv_string_slice

jq/src/jv.c

Line 1374 in c95b34f

jv jv_string_slice(jv j, int start, int end) {

i'm not sure anymore if one can assume strings are valid utf-8 or is the invalid utf-8 checks not really needed?

[itchyny]

I'd like to include this. Any objection on changing the behavior in 1.8?

[wader]

Ok to merge for me but would be great if someone could have a look or know if my assumption about strings always being valid utf-8 is true.

[pkoppstein]

@itchyny asked:

Any objection on changing the behavior in 1.8

This is a major breaking change and it has been my understanding for some years that such changes would have to wait until jq 2.0. Certainly if we were following a strict SemVer policy that would be the case. Since we don't seem to be doing so, the situation is not black-and-white, but if the change is incorporated into 1.8, we should be sure to highlight it.

@wader wrote:

if my assumption about strings always being valid utf-8 is true.

Based on past experience, such an assumption would not be warranted, so the question is: could the proposed changes make anything worse? I suppose the major issue would be whether (in the presence of invalid utf-8) the old index would give an accurate byte count but the new version might give an inaccurate codepoint count.

Perhaps a starting point would be "a\uDD1Ec":

echo '"a\uDD1Ec"' | jaq -r .
Error: failed to parse: invalid character with index 56606

echo '"a\uDD1Ec"' | jq -c '[index("c"), length]'
[4,3]

[wader]

@itchyny asked:

Any objection on changing the behavior in 1.8

This is a major breaking change and it has been my understanding for some years that such changes would have to wait until jq 2.0. Certainly if we were following a strict SemVer policy that would be the case. Since we don't seem to be doing so, the situation is not black-and-white, but if the change is incorporated into 1.8, we should be sure to highlight it.

I can't see how the current behaviour for non-ASCII strings makes any sense or could even be useful in any resonable way? so for me it feels more like a bug.

@wader wrote:

if my assumption about strings always being valid utf-8 is true.

Based on past experience, such an assumption would not be warranted, so the question is: could the proposed changes make anything worse? I suppose the major issue would be whether (in the presence of invalid utf-8) the old index would give an accurate byte count but the new version might give an inaccurate codepoint count.

Perhaps a starting point would be "a\uDD1Ec":

echo '"a\uDD1Ec"' | jaq -r .
Error: failed to parse: invalid character with index 56606

echo '"a\uDD1Ec"' | jq -c '[index("c"), length]'
[4,3]

This is an incomplete surrogates pair? yeap stuff like this i'm concerned about also.

[wader]

With this change:

$ echo '"a\uDD1Ec"' | ./jq -c '[index("c"), length]'
[2,3]

Seems correct assuming broken surrogates codepoints should be allowed. But I think i'm mostly concern if there is any way to produce jq strings that has a byte buffer that is not valid utf-8. If so use of jvp_utf8_decode_length might end up out-of-sync codepoint-wise or pointing outside the byte buffer.