vis: fix search wrapping bugs

[zsugabubus]

See commit message.

[martanne]

Thanks, there is a more general problem though. The regex anchors ^ and $ match the start/end of the search range irrespective of whether they are at a newline or not. I guess this could be fixed by supplying REG_NOTBOL and REG_NOTEOL when needed?

[zsugabubus]

My first thought was that, but think about what if:

• It's end-of-line indeed because the next character is \n. That would require an extra check.
• The possible match would span through the two parts. Then it's not a good idea to stop before the second half of the string.

For me it appeared to be the simplest solution, everything other would make the handling of this/these edge-case(s) unnecessarily complicated. And it goes through the text (absolutely at most) twice only if no matches found at all.

[martanne]

I agree that it is more complex and requires additional checks. However, currently the anchors match immediately even when they shouldn't. For example searching for ^vis in the following is mishandled (#denotes the cursor/selection):

vi#vis
visvis

An analogous problem exists for backward searches with $.

[zsugabubus]

Give it a try: vis/fix-search-anchors. It seems it's working with POSIX regexps, but I didn't test it with --enable-tre.

[martanne]

Thanks, it seems more tricky than I initially thought. I think the logic about the REG_* flags belongs in the caller. I pushed some changes, let me know whether it works for you? It was only briefly tested.

[zsugabubus]

1. Not sure if it's related, but I found another anomaly (just before I wanted to tell you that I found everything okay) around zero-width backwards searches:

vis
vis|

(Works with any text.)

Try /$, N.

2. If I understand right, skipping “junks” in text_search_range_{back,for}ward() is needed because regexec() expects a zero-terminated string and it requires special handling.
   It can cause issues in cases where REG_NOTEOL was not supplied because then it will give a match for $ right before the zero chunk (and ^ can give a match right after the chunk).

I also found that advancing to the next non-zero char could be simplified to:

/* No need for `len` and `junk`. */
while (cur != end) {
  ...
  /* rawmemchr() is non-standard but we already require _GNU_SOURCE because of memrchr() */
  cur = rawmemchr(cur, '\0'); /* Knowing that buf is always zero-terminated. */
  while (*cur == '\0' && cur < end)
    ++cur;
  eflags |= REG_NOTBOL; /* Surely not line beginning if the previous char was NUL. */
}

[martanne]

Thanks, point 1. should be fixed by b416a91?

And yes searching in binary files, will still show some oddities.