Subject: bug#37488: 27.0.50; Inconsistent naming of arg of `text-property-search-forward' and `-backward'

From: Lars Ingebrigtsen <[email-masked]>
Date: Thu, 26 Sep 2019 17:12:46 +0200

Eli Zaretskii <[email-masked]> writes:

>> >  . The original doc string said about nil as PREDICATE:
>> >
>> >      `nil' means almost the same as \"not equal\", but will also
>> >      end the match if the value of PROPERTY changes.
>> >
>> >    What does it mean to "end the match if the value changes"?  The
>> >    text went on to say "See the manual for extensive examples", but
>> >    the manual doesn't mention this aspect of nil, it just says "not
>> >    equal".
>> 
>> If point is inside a region where PROPERTY has a non-nil value, using
>> nil as the predicate will end the search when PROPERTY changes value.
>
> OK, but then it should be obvious, and explicitly mentioning that in
> the doc string just makes it more confusing.  Or am I missing
> something?

It's not obvious -- it's the difference between nil and a lambda that
just says (not (equal...)).

I think the manual needs those copious examples that I apparently forgot
to write.

It's probably easier to understand if you go to a buffer like this:

(defun text-property-setup ()
  (insert "This is "
          (propertize "bold1" 'face 'bold)
          " and this is "
          (propertize "italic1" 'face 'italic)
          (propertize "bold2" 'face 'bold)
          (propertize "italic2" 'face 'italic)
          " at the end")
  (goto-char (point-min)))

and see the difference between 

(text-property-search-forward 'face 'bold nil)

and

(text-property-search-forward 'face 'bold
                              (lambda (val p-val) (not (equal val p-val))))

The latter will stop every time 'face changes, while the former stops in
a way that segments the buffer in all the regions that have not 'face
'bold.

>> >  . The doc string stated:
>> >
>> >      If NOT-CURRENT, if the match is under point, it will not be
>> >      returned, but instead the next instance is returned, if any.
>> >
>> >    But the code, AFAIU, just checks whether the value of PROPERTY at
>> >    point matches that of the found region, which doesn't necessarily
>> >    mean point is part of the region, it could just mean there are two
>> >    regions with matching values of PROPERTY, and point belongs to one
>> >    of them.  Right?
>> 
>> I'm not sure I understand the question...
>
> The code doesn't seem to make sure point is inside the same region as
> the one found by the initial attempt, it just compares the values of
> PROPERTY at point and in the found region, and if they match, it looks
> for another region.  Is that a correct description of what the code
> does when NOT-CURRENT is non-nil?

Yes.

> If the description is correct, then the doc string doesn't describe
> what the code does accurately.

I think that's what the current doc string also says, but perhaps not
clearly enough.

>> What it's meant to do is basically how TAB works in buttons, which is a
>> common use case.  If you're looking for "o", and point is where "|" is
>> below, then it'll find the last "oo" section after the x-es:
>> 
>> oo|oxxxxxoo
>
> What does "o" stand for in this case?

Text with "o" as the property.

>> >  . What is the reason for having VALUE an optional argument?  Is it a
>> >    frequent/useful operation to look for a VALUE of nil?
>> 
>> If PREDICATE is nil, then having to use an explicit nil VALUE isn't
>> necessary.
>
> I don't understand how this answers my questions.

Try (text-property-search-forward 'face) on the example.  It'll give you
all regions that has a face property.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no