[PATCH] Re: [V] Fix search assertion failure

[Stephen J. Turnbull]

Aidan Kehoe writes:

 >  > So maybe the code works for you, but it's not working according to
 >  > theory.
 >  > 
 >  > At the least, I think the assert should be replaced with
 >  > 
 >  >     if (starting_ch != translated) { boyer_moore_ok = 0; break; }
 >  > 
 >  > I believe that's safe.  Aidan?
 > 
 > No, the assertion is well-founded when it comes to the algorithms used.

Sorry, I didn't mean to say otherwise.  What I meant was my suggestion
would give correct results but at the expense of not implementing the
algorithm you had in mind.

 > It doesn’t take into account the (new) feature characters may have
 > been ignored because they were unrepresentable in the buffer

Urk.  What does that mean?  "Unrepresentable" is not a buffer-local
property, it is a global property of the Mule encoding.  Characters
are either representable or they are not, and that's the same for
character, string, and buffer objects.  Coding systems have nothing to
do with possible buffer content AFAIK.  This would change if we
implemented fixed-width buffers of course.

 > The below patch fails immediately when it encounters characters in the
 > search string that are not possible in the buffer, and avoids this
 > assertion.

By that you mean no search is executed but you return failure?  That
can't work because there's no a priori way to know what characters are
in the buffer AFAIK.

 > +(with-temp-buffer
 > +  (Assert (search-forward "M\xe9zard" nil t)))

Shouldn't this be

  (with-temp-buffer
    (Assert (not (search-forward "M\xe9zard" nil t))))

since `search-forward' should return nil for any non-degenerate search
in an empty buffer?

Please add at least one success case to the test, eg:

  (with-temp-buffer
    (let ((target "M\xe9zard"))
      (Assert (not (search-forward target nil t)))
      (insert target)
      (goto-char (point-min))
      (Assert (= 1 (search-forward target nil t)))))

(Test untested.)