unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#23937: 25.0.95; Search functions doc fixes/improvements
@ 2016-07-10 18:21 Stephen Berman
  2016-07-11 15:20 ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Berman @ 2016-07-10 18:21 UTC (permalink / raw)
  To: 23937

In GNU Emacs 25.0.95.9 (x86_64-suse-linux-gnu, GTK+ Version 3.14.15)
 of 2016-07-10 built on rosalinde
Repository revision: 4069b716ad3422f2d7f595699220c39297427387

Bug#10507 was about missing documentation for the fact that the fourth
argument of search-{for,back}ward can be a negative number, reversing
the search direction.  This lack was fixed in commit acc28cb, but that
commit didn't correspondingly augment the doc of the other search
functions (word-search-*, re-search-*, posix-search-*), which have the
same behavior.  The patch below does this.  In addition, the
documentation in the Lisp reference uses the word "repeat" instead of
"count" for that argument, and says the search is repeated N times,
where N is the argument's value; but strictly speaking, it's repeated
N-1 times.  It may be clear what's intended, but there's no need for the
manual to differ from (and be strictly less accurate than) the doc
strings on this point, so the patch changes the manual accordingly.
Finally, the patch also adds the information about the nil value of the
bound/limit argument to those doc strings/manual entries where it's
missing.  In short the patch makes the doc of all these functions more
uniform.

diff --git a/doc/lispref/searching.texi b/doc/lispref/searching.texi
index 1243d72..8d0e4ca 100644
--- a/doc/lispref/searching.texi
+++ b/doc/lispref/searching.texi
@@ -44,7 +44,7 @@ String Search
 buffer is multibyte; they convert the search string to unibyte if the
 buffer is unibyte.  @xref{Text Representations}.
 
-@deffn Command search-forward string &optional limit noerror repeat
+@deffn Command search-forward string &optional limit noerror count
 This function searches forward from point for an exact match for
 @var{string}.  If successful, it sets point to the end of the occurrence
 found, and returns the new value of point.  If no match is found, the
@@ -95,24 +95,24 @@ String Search
 find a match.  Invalid arguments cause errors regardless of
 @var{noerror}.
 
-If @var{repeat} is a positive number @var{n}, it serves as a repeat
-count: the search is repeated @var{n} times, each time starting at the
-end of the previous time's match.  If these successive searches
-succeed, the function succeeds, moving point and returning its new
-value.  Otherwise the search fails, with results depending on the
-value of @var{noerror}, as described above.  If @var{repeat} is a
-negative number -@var{n}, it serves as a repeat count of @var{n} for a
-search in the opposite (backward) direction.
+If @var{count} is a positive number @var{n}, the search is done
+@var{n} times; each repetition starts at the end of the previous
+match.  If all these successive searches succeed, the function call
+succeeds, moving point and returning its new value.  Otherwise the
+function call fails, with results depending on the value of
+@var{noerror}, as described above.  If @var{count} is a negative
+number -@var{n}, the search is done @var{n} times in the opposite
+(backward) direction.
 @end deffn
 
-@deffn Command search-backward string &optional limit noerror repeat
+@deffn Command search-backward string &optional limit noerror count
 This function searches backward from point for @var{string}.  It is
 like @code{search-forward}, except that it searches backwards rather
 than forwards.  Backward searches leave point at the beginning of the
 match.
 @end deffn
 
-@deffn Command word-search-forward string &optional limit noerror repeat
+@deffn Command word-search-forward string &optional limit noerror count
 This function searches forward from point for a word match for
 @var{string}.  If it finds a match, it sets point to the end of the
 match found, and returns the new value of point.
@@ -156,8 +156,10 @@ String Search
 neither @code{nil} nor @code{t}, it moves point to @var{limit} (or the
 end of the accessible portion of the buffer) and returns @code{nil}.
 
-If @var{repeat} is non-@code{nil}, then the search is repeated that many
-times.  Point is positioned at the end of the last match.
+If @var{count} is a positive number, it specifies how many successive
+occurrences to search for.  Point is positioned at the end of the last
+match.  If @var{count} is a negative number, the search is backward
+and point is positioned at the beginning of the last match.
 
 @findex word-search-regexp
 Internally, @code{word-search-forward} and related functions use the
@@ -165,7 +167,7 @@ String Search
 regular expression that ignores punctuation.
 @end deffn
 
-@deffn Command word-search-forward-lax string &optional limit noerror repeat
+@deffn Command word-search-forward-lax string &optional limit noerror count
 This command is identical to @code{word-search-forward}, except that
 the beginning or the end of @var{string} need not match a word
 boundary, unless @var{string} begins or ends in whitespace.
@@ -173,14 +175,14 @@ String Search
 but does not match @samp{balls boy}.
 @end deffn
 
-@deffn Command word-search-backward string &optional limit noerror repeat
+@deffn Command word-search-backward string &optional limit noerror count
 This function searches backward from point for a word match to
 @var{string}.  This function is just like @code{word-search-forward}
 except that it searches backward and normally leaves point at the
 beginning of the match.
 @end deffn
 
-@deffn Command word-search-backward-lax string &optional limit noerror repeat
+@deffn Command word-search-backward-lax string &optional limit noerror count
 This command is identical to @code{word-search-backward}, except that
 the beginning or the end of @var{string} need not match a word
 boundary, unless @var{string} begins or ends in whitespace.
@@ -1005,7 +1007,7 @@ Regexp Search
 the buffer is multibyte; they convert the regular expression to unibyte
 if the buffer is unibyte.  @xref{Text Representations}.
 
-@deffn Command re-search-forward regexp &optional limit noerror repeat
+@deffn Command re-search-forward regexp &optional limit noerror count
 This function searches forward in the current buffer for a string of
 text that is matched by the regular expression @var{regexp}.  The
 function skips over any amount of text that is not matched by
@@ -1014,14 +1016,12 @@ Regexp Search
 
 If @var{limit} is non-@code{nil}, it must be a position in the current
 buffer.  It specifies the upper bound to the search.  No match
-extending after that position is accepted.
+extending after that position is accepted.  If @var{limit} is omitted
+or @code{nil}, it defaults to the end of the accessible portion of the
+buffer.
 
-If @var{repeat} is supplied, it must be a positive number; the search
-is repeated that many times; each repetition starts at the end of the
-previous match.  If all these successive searches succeed, the search
-succeeds, moving point and returning its new value.  Otherwise the
-search fails.  What @code{re-search-forward} does when the search
-fails depends on the value of @var{noerror}:
+What @code{re-search-forward} does when the search fails depends on
+the value of @var{noerror}:
 
 @table @asis
 @item @code{nil}
@@ -1033,6 +1033,19 @@ Regexp Search
 buffer) and return @code{nil}.
 @end table
 
+The argument @var{noerror} only affects valid searches which fail to
+find a match.  Invalid arguments cause errors regardless of
+@var{noerror}.
+
+If @var{count} is a positive number @var{n}, the search is done
+@var{n} times; each repetition starts at the end of the previous
+match.  If all these successive searches succeed, the function call
+succeeds, moving point and returning its new value.  Otherwise the
+function call fails, with results depending on the value of
+@var{noerror}, as described above.  If @var{count} is a negative
+number -@var{n}, the search is done @var{n} times in the opposite
+(backward) direction.
+
 In the following example, point is initially before the @samp{T}.
 Evaluating the search call moves point to the end of that line (between
 the @samp{t} of @samp{hat} and the newline).
@@ -1057,7 +1070,7 @@ Regexp Search
 @end example
 @end deffn
 
-@deffn Command re-search-backward regexp &optional limit noerror repeat
+@deffn Command re-search-backward regexp &optional limit noerror count
 This function searches backward in the current buffer for a string of
 text that is matched by the regular expression @var{regexp}, leaving
 point at the beginning of the first text found.
@@ -1228,13 +1241,13 @@ POSIX Regexps
 This is because POSIX backtracking conflicts with the semantics of
 non-greedy repetition.
 
-@deffn Command posix-search-forward regexp &optional limit noerror repeat
+@deffn Command posix-search-forward regexp &optional limit noerror count
 This is like @code{re-search-forward} except that it performs the full
 backtracking specified by the POSIX standard for regular expression
 matching.
 @end deffn
 
-@deffn Command posix-search-backward regexp &optional limit noerror repeat
+@deffn Command posix-search-backward regexp &optional limit noerror count
 This is like @code{re-search-backward} except that it performs the full
 backtracking specified by the POSIX standard for regular expression
 matching.
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 7360a0b..322f2aa 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1627,7 +1627,8 @@ word-search-backward
 The match found must not extend before that position.
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument specifies how many successive occurrences to
+  search for.
 
 Relies on the function `word-search-regexp' to convert a sequence
 of words in STRING to a regexp used to search words without regard
@@ -1644,7 +1645,8 @@ word-search-forward
 The match found must not extend after that position.
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument specifies how many successive occurrences to
+  search for.
 
 Relies on the function `word-search-regexp' to convert a sequence
 of words in STRING to a regexp used to search words without regard
@@ -1665,7 +1667,8 @@ word-search-backward-lax
 The match found must not extend before that position.
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument specifies how many successive occurrences to
+  search for.
 
 Relies on the function `word-search-regexp' to convert a sequence
 of words in STRING to a regexp used to search words without regard
@@ -1686,7 +1689,8 @@ word-search-forward-lax
 The match found must not extend after that position.
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument specifies how many successive occurrences to
+  search for.
 
 Relies on the function `word-search-regexp' to convert a sequence
 of words in STRING to a regexp used to search words without regard
diff --git a/src/search.c b/src/search.c
index bcdd8f1..6614781 100644
--- a/src/search.c
+++ b/src/search.c
@@ -2164,7 +2164,8 @@ DEFUN ("search-backward", Fsearch_backward, Ssearch_backward, 1, 4,
        doc: /* Search backward from point for STRING.
 Set point to the beginning of the occurrence found, and return point.
 An optional second argument bounds the search; it is a buffer position.
-The match found must not extend before that position.
+The match found must not extend before that position.  A value of nil is
+  equivalent to (point-min).
 Optional third argument, if t, means if fail just return nil (no error).
  If not nil and not t, position at limit of search and return nil.
 Optional fourth argument COUNT, if non-nil, means to search for COUNT
@@ -2204,14 +2205,15 @@ See also the functions `match-beginning', `match-end' and `replace-match'.  */)
 DEFUN ("re-search-backward", Fre_search_backward, Sre_search_backward, 1, 4,
        "sRE search backward: ",
        doc: /* Search backward from point for match for regular expression REGEXP.
-Set point to the beginning of the match, and return point.
-The match found is the one starting last in the buffer
-and yet ending before the origin of the search.
+Set point to the beginning of the occurrence found, and return point.
 An optional second argument bounds the search; it is a buffer position.
-The match found must start at or after that position.
+The match found must not extend before that position.  A value of nil is
+  equivalent to (point-min).
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument COUNT, if non-nil, means to search for COUNT
+ successive occurrences.  If COUNT is negative, search forward,
+ instead of backward, for -COUNT occurrences.
 
 Search case-sensitivity is determined by the value of the variable
 `case-fold-search', which see.
@@ -2228,10 +2230,13 @@ DEFUN ("re-search-forward", Fre_search_forward, Sre_search_forward, 1, 4,
        doc: /* Search forward from point for regular expression REGEXP.
 Set point to the end of the occurrence found, and return point.
 An optional second argument bounds the search; it is a buffer position.
-The match found must not extend after that position.
+The match found must not extend after that position.  A value of nil is
+  equivalent to (point-max).
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument COUNT, if non-nil, means to search for COUNT
+ successive occurrences.  If COUNT is negative, search backward,
+ instead of forward, for -COUNT occurrences.
 
 Search case-sensitivity is determined by the value of the variable
 `case-fold-search', which see.
@@ -2247,14 +2252,15 @@ DEFUN ("posix-search-backward", Fposix_search_backward, Sposix_search_backward,
        "sPosix search backward: ",
        doc: /* Search backward from point for match for regular expression REGEXP.
 Find the longest match in accord with Posix regular expression rules.
-Set point to the beginning of the match, and return point.
-The match found is the one starting last in the buffer
-and yet ending before the origin of the search.
+Set point to the beginning of the occurrence found, and return point.
 An optional second argument bounds the search; it is a buffer position.
-The match found must start at or after that position.
+The match found must start at or after that position.  A value of nil is
+  equivalent to (point-min).
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument COUNT, if non-nil, means to search for COUNT
+ successive occurrences.  If COUNT is negative, search forward,
+ instead of backward, for -COUNT occurrences.
 
 Search case-sensitivity is determined by the value of the variable
 `case-fold-search', which see.
@@ -2272,10 +2278,13 @@ DEFUN ("posix-search-forward", Fposix_search_forward, Sposix_search_forward, 1,
 Find the longest match in accord with Posix regular expression rules.
 Set point to the end of the occurrence found, and return point.
 An optional second argument bounds the search; it is a buffer position.
-The match found must not extend after that position.
+The match found must not extend after that position.  A value of nil is
+  equivalent to (point-max).
 Optional third argument, if t, means if fail just return nil (no error).
   If not nil and not t, move to limit of search and return nil.
-Optional fourth argument is repeat count--search for successive occurrences.
+Optional fourth argument COUNT, if non-nil, means to search for COUNT
+ successive occurrences.  If COUNT is negative, search backward,
+ instead of forward, for -COUNT occurrences.
 
 Search case-sensitivity is determined by the value of the variable
 `case-fold-search', which see.





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-10 18:21 bug#23937: 25.0.95; Search functions doc fixes/improvements Stephen Berman
@ 2016-07-11 15:20 ` Eli Zaretskii
  2016-07-11 21:55   ` Stephen Berman
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2016-07-11 15:20 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 23937

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Sun, 10 Jul 2016 20:21:34 +0200
> 
> In GNU Emacs 25.0.95.9 (x86_64-suse-linux-gnu, GTK+ Version 3.14.15)
>  of 2016-07-10 built on rosalinde
> Repository revision: 4069b716ad3422f2d7f595699220c39297427387
> 
> Bug#10507 was about missing documentation for the fact that the fourth
> argument of search-{for,back}ward can be a negative number, reversing
> the search direction.  This lack was fixed in commit acc28cb, but that
> commit didn't correspondingly augment the doc of the other search
> functions (word-search-*, re-search-*, posix-search-*), which have the
> same behavior.

(But no one has documented what happens if COUNT is zero... ;-)

> The patch below does this.  In addition, the
> documentation in the Lisp reference uses the word "repeat" instead of
> "count" for that argument, and says the search is repeated N times,
> where N is the argument's value; but strictly speaking, it's repeated
> N-1 times.  It may be clear what's intended, but there's no need for the
> manual to differ from (and be strictly less accurate than) the doc
> strings on this point, so the patch changes the manual accordingly.
> Finally, the patch also adds the information about the nil value of the
> bound/limit argument to those doc strings/manual entries where it's
> missing.  In short the patch makes the doc of all these functions more
> uniform.

Thanks.  I have some minor comments.

> +If @var{count} is a positive number @var{n}, the search is done
> +@var{n} times; each repetition starts at the end of the previous
                       ^^^^^^^^^^
With the change in the name of the argument, there's no more
"repetitions".  Suggest to use "successive search" instead.

> +If @var{count} is a positive number @var{n}, the search is done
> +@var{n} times; each repetition starts at the end of the previous
                       ^^^^^^^^^^
Likewise.

> +The match found must not extend before that position.  A value of nil is
> +  equivalent to (point-min).

I find references to point-min and point-max in doc strings not the
best style.  I think saying "the search is unlimited" or "defaults to
the beginning of the buffer's accessible region" is more
user-friendly.

Btw, since this is search backwards, the "extend" part might be
confusing; perhaps it is better to say "match must not begin before
that position" instead?

> @@ -2247,14 +2252,15 @@ DEFUN ("posix-search-backward", Fposix_search_backward, Sposix_search_backward,
>         "sPosix search backward: ",
>         doc: /* Search backward from point for match for regular expression REGEXP.
>  Find the longest match in accord with Posix regular expression rules.
> -Set point to the beginning of the match, and return point.
> -The match found is the one starting last in the buffer
> -and yet ending before the origin of the search.
> +Set point to the beginning of the occurrence found, and return point.

The 2nd and 3rd lines you removed seem to provide valuable information
which is now gone, no?

With those taken care of, LGTM, thanks.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-11 15:20 ` Eli Zaretskii
@ 2016-07-11 21:55   ` Stephen Berman
  2016-07-12  5:03     ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Berman @ 2016-07-11 21:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23937

On Mon, 11 Jul 2016 18:20:51 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Date: Sun, 10 Jul 2016 20:21:34 +0200
>> 
>> In GNU Emacs 25.0.95.9 (x86_64-suse-linux-gnu, GTK+ Version 3.14.15)
>>  of 2016-07-10 built on rosalinde
>> Repository revision: 4069b716ad3422f2d7f595699220c39297427387
>> 
>> Bug#10507 was about missing documentation for the fact that the fourth
>> argument of search-{for,back}ward can be a negative number, reversing
>> the search direction.  This lack was fixed in commit acc28cb, but that
>> commit didn't correspondingly augment the doc of the other search
>> functions (word-search-*, re-search-*, posix-search-*), which have the
>> same behavior.
>
> (But no one has documented what happens if COUNT is zero... ;-)

In all seriousness, I actually considered adding that, but then thought
it's not really needed, since COUNT means do the search that many times,
so doing it zero times means nothing happens, which is exactly the
case.  (But we could add it, see below.)

>> The patch below does this.  In addition, the
>> documentation in the Lisp reference uses the word "repeat" instead of
>> "count" for that argument, and says the search is repeated N times,
>> where N is the argument's value; but strictly speaking, it's repeated
>> N-1 times.  It may be clear what's intended, but there's no need for the
>> manual to differ from (and be strictly less accurate than) the doc
>> strings on this point, so the patch changes the manual accordingly.
>> Finally, the patch also adds the information about the nil value of the
>> bound/limit argument to those doc strings/manual entries where it's
>> missing.  In short the patch makes the doc of all these functions more
>> uniform.
>
> Thanks.  I have some minor comments.
>
>> +If @var{count} is a positive number @var{n}, the search is done
>> +@var{n} times; each repetition starts at the end of the previous
>                        ^^^^^^^^^^
> With the change in the name of the argument, there's no more
> "repetitions".  Suggest to use "successive search" instead.

To my understanding, with COUNT > 1 all searches but the initial one are
repetitions, but it's fine with me to use "successive search".

>> +If @var{count} is a positive number @var{n}, the search is done
>> +@var{n} times; each repetition starts at the end of the previous
>                        ^^^^^^^^^^
> Likewise.
>
>> +The match found must not extend before that position.  A value of nil is
>> +  equivalent to (point-min).
>
> I find references to point-min and point-max in doc strings not the
> best style.  I think saying "the search is unlimited" or "defaults to
> the beginning of the buffer's accessible region" is more
> user-friendly.
>
> Btw, since this is search backwards, the "extend" part might be
> confusing; perhaps it is better to say "match must not begin before
> that position" instead?

Sure, both of these are fine with me.

>> @@ -2247,14 +2252,15 @@ DEFUN ("posix-search-backward", Fposix_search_backward, Sposix_search_backward,
>>         "sPosix search backward: ",
>>         doc: /* Search backward from point for match for regular expression REGEXP.
>>  Find the longest match in accord with Posix regular expression rules.
>> -Set point to the beginning of the match, and return point.
>> -The match found is the one starting last in the buffer
>> -and yet ending before the origin of the search.
>> +Set point to the beginning of the occurrence found, and return point.
>
> The 2nd and 3rd lines you removed seem to provide valuable information
> which is now gone, no?

I removed them because I thought they were (i) partly redundant: the
first words of the doc string, "Search backward from point", already
imply that the match ends before that position, since otherwise it
wouldn't be a search backward; and (ii) partly inaccurate: saying the
match found is the last one in the buffer before the starting point is
only true when COUNT is 1 (or nil).  A sufficient and more accurate
statement would in effect describe the COUNT argument, which the doc
string already does.  So I don't see the need for those two lines.  But
we could add the meaning of nil to the description of COUNT for
explicitness, and 0 for completeness (or not, if you think it's not
worth it):

   Optional fourth argument COUNT, if a positive number, means to search
    for COUNT successive occurrences.  If COUNT is negative, search
    forward, instead of backward, for -COUNT occurrences.  A value of
    nil means the same as 1, and 0 means do nothing.

> With those taken care of, LGTM, thanks.

Thanks for reviewing.  I'll wait for your final decision on the last
point, and then I assume it's ok to push the changes to emacs-25?

Steve Berman





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-11 21:55   ` Stephen Berman
@ 2016-07-12  5:03     ` Eli Zaretskii
  2016-07-12 12:26       ` Stephen Berman
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2016-07-12  5:03 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 23937

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 23937@debbugs.gnu.org
> Date: Mon, 11 Jul 2016 23:55:19 +0200
> 
> >> @@ -2247,14 +2252,15 @@ DEFUN ("posix-search-backward", Fposix_search_backward, Sposix_search_backward,
> >>         "sPosix search backward: ",
> >>         doc: /* Search backward from point for match for regular expression REGEXP.
> >>  Find the longest match in accord with Posix regular expression rules.
> >> -Set point to the beginning of the match, and return point.
> >> -The match found is the one starting last in the buffer
> >> -and yet ending before the origin of the search.
> >> +Set point to the beginning of the occurrence found, and return point.
> >
> > The 2nd and 3rd lines you removed seem to provide valuable information
> > which is now gone, no?
> 
> I removed them because I thought they were (i) partly redundant: the
> first words of the doc string, "Search backward from point", already
> imply that the match ends before that position, since otherwise it
> wouldn't be a search backward

I don't agree, I think this part is not clear unless explicitly
described.  I frequently find myself wondering about this when I
need to code a loop that uses these functions.

> and (ii) partly inaccurate: saying the
> match found is the last one in the buffer before the starting point is
> only true when COUNT is 1 (or nil)

Then let's make it more accurate, but I don't think removing that is
TRT.  In particular, there's nothing wrong with describing what
happens with COUNT = 1 if the clarity is required only in that case.

> I assume it's ok to push the changes to emacs-25?

Yes.

(The comment about COUNT = 0 was a teaser, don't worry about that.)





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-12  5:03     ` Eli Zaretskii
@ 2016-07-12 12:26       ` Stephen Berman
  2016-07-12 13:10         ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Berman @ 2016-07-12 12:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23937

On Tue, 12 Jul 2016 08:03:25 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: 23937@debbugs.gnu.org
>> Date: Mon, 11 Jul 2016 23:55:19 +0200
>> 
>> >> @@ -2247,14 +2252,15 @@ DEFUN ("posix-search-backward", Fposix_search_backward, Sposix_search_backward,
>> >>         "sPosix search backward: ",
>> >>         doc: /* Search backward from point for match for regular expression REGEXP.
>> >>  Find the longest match in accord with Posix regular expression rules.
>> >> -Set point to the beginning of the match, and return point.
>> >> -The match found is the one starting last in the buffer
>> >> -and yet ending before the origin of the search.
>> >> +Set point to the beginning of the occurrence found, and return point.
>> >
>> > The 2nd and 3rd lines you removed seem to provide valuable information
>> > which is now gone, no?
>> 
>> I removed them because I thought they were (i) partly redundant: the
>> first words of the doc string, "Search backward from point", already
>> imply that the match ends before that position, since otherwise it
>> wouldn't be a search backward
>
> I don't agree, I think this part is not clear unless explicitly
> described.  I frequently find myself wondering about this when I
> need to code a loop that uses these functions.
>
>> and (ii) partly inaccurate: saying the
>> match found is the last one in the buffer before the starting point is
>> only true when COUNT is 1 (or nil)
>
> Then let's make it more accurate, but I don't think removing that is
> TRT.  In particular, there's nothing wrong with describing what
> happens with COUNT = 1 if the clarity is required only in that case.

How about the following, which moves the two lines, rephrased for
accuracy, to the end of the doc string?

  Set point to the beginning of the occurrence found, and return point.
  An optional second argument bounds the search; it is a buffer position.
    The match found must start at or after that position.  A value of nil
    means search to the beginning of the accessible portion of the buffer.
  Optional third argument, if t, means if fail just return nil (no error).
    If not nil and not t, move to limit of search and return nil.
  Optional fourth argument COUNT, if a positive number, means to search
    for COUNT successive occurrences.  If COUNT is negative, search
    forward, instead of backward, for -COUNT occurrences.  A value of
    nil means the same as 1.
  The match found is the COUNTth to last one (or last, if COUNT is 1 or
    nil) in the buffer located entirely before the origin of the search.

If you are ok with this, should I add these two lines to all
*search-backward and (suitably adapted) *search-forward functions?  (The
two lines are currently only in {re,posix}-search-backward.)

Steve Berman





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-12 12:26       ` Stephen Berman
@ 2016-07-12 13:10         ` Eli Zaretskii
  2016-07-12 15:14           ` Stephen Berman
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2016-07-12 13:10 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 23937

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 23937@debbugs.gnu.org
> Date: Tue, 12 Jul 2016 14:26:12 +0200
> 
>   Set point to the beginning of the occurrence found, and return point.
>   An optional second argument bounds the search; it is a buffer position.
>     The match found must start at or after that position.  A value of nil
>     means search to the beginning of the accessible portion of the buffer.
>   Optional third argument, if t, means if fail just return nil (no error).
>     If not nil and not t, move to limit of search and return nil.
>   Optional fourth argument COUNT, if a positive number, means to search
>     for COUNT successive occurrences.  If COUNT is negative, search
>     forward, instead of backward, for -COUNT occurrences.  A value of
>     nil means the same as 1.
>   The match found is the COUNTth to last one (or last, if COUNT is 1 or
>     nil) in the buffer located entirely before the origin of the search.

LGTM, thanks.

> If you are ok with this, should I add these two lines to all
> *search-backward and (suitably adapted) *search-forward functions?  (The
> two lines are currently only in {re,posix}-search-backward.)

It's better for all those doc strings to be consistent, yes.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-12 13:10         ` Eli Zaretskii
@ 2016-07-12 15:14           ` Stephen Berman
  2016-07-12 15:23             ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Berman @ 2016-07-12 15:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23937, Stephen Berman

On Tue, 12 Jul 2016 16:10:28 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: 23937@debbugs.gnu.org
>> Date: Tue, 12 Jul 2016 14:26:12 +0200
>> 
>>   Set point to the beginning of the occurrence found, and return point.
>>   An optional second argument bounds the search; it is a buffer position.
>>     The match found must start at or after that position.  A value of nil
>>     means search to the beginning of the accessible portion of the buffer.
>>   Optional third argument, if t, means if fail just return nil (no error).
>>     If not nil and not t, move to limit of search and return nil.
>>   Optional fourth argument COUNT, if a positive number, means to search
>>     for COUNT successive occurrences.  If COUNT is negative, search
>>     forward, instead of backward, for -COUNT occurrences.  A value of
>>     nil means the same as 1.
>>   The match found is the COUNTth to last one (or last, if COUNT is 1 or
>>     nil) in the buffer located entirely before the origin of the search.
>
> LGTM, thanks.
>
>> If you are ok with this, should I add these two lines to all
>> *search-backward and (suitably adapted) *search-forward functions?  (The
>> two lines are currently only in {re,posix}-search-backward.)
>
> It's better for all those doc strings to be consistent, yes.

Oh, dear.  I made all the changes and was ready to commit them, when I
realized that those final two lines are only valid for positive COUNT.
Spelling it out for negative COUNT seems like overkill; how about this:

   With COUNT positive, the match found is the COUNTth to last one (or
     last, if COUNT is 1 or nil) in the buffer located entirely before
     the origin of the search; correspondingly with COUNT negative.

I hope this is the last point of this issue that needs clarifying.

Steve Berman





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-12 15:14           ` Stephen Berman
@ 2016-07-12 15:23             ` Eli Zaretskii
  2016-07-12 20:13               ` Stephen Berman
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2016-07-12 15:23 UTC (permalink / raw)
  To: Stephen Berman; +Cc: 23937

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: Stephen Berman <stephen.berman@gmx.net>,  23937@debbugs.gnu.org
> Date: Tue, 12 Jul 2016 17:14:19 +0200
> 
>    With COUNT positive, the match found is the COUNTth to last one (or
>      last, if COUNT is 1 or nil) in the buffer located entirely before
>      the origin of the search; correspondingly with COUNT negative.

OK.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#23937: 25.0.95; Search functions doc fixes/improvements
  2016-07-12 15:23             ` Eli Zaretskii
@ 2016-07-12 20:13               ` Stephen Berman
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Berman @ 2016-07-12 20:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Stephen Berman, 23937-done

On Tue, 12 Jul 2016 18:23:26 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: Stephen Berman <stephen.berman@gmx.net>,  23937@debbugs.gnu.org
>> Date: Tue, 12 Jul 2016 17:14:19 +0200
>> 
>>    With COUNT positive, the match found is the COUNTth to last one (or
>>      last, if COUNT is 1 or nil) in the buffer located entirely before
>>      the origin of the search; correspondingly with COUNT negative.
>
> OK.

Pushed to emacs-25 as commit 069fc05 and closing the bug; thanks for the
feedback.

Steve Berman





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-07-12 20:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-10 18:21 bug#23937: 25.0.95; Search functions doc fixes/improvements Stephen Berman
2016-07-11 15:20 ` Eli Zaretskii
2016-07-11 21:55   ` Stephen Berman
2016-07-12  5:03     ` Eli Zaretskii
2016-07-12 12:26       ` Stephen Berman
2016-07-12 13:10         ` Eli Zaretskii
2016-07-12 15:14           ` Stephen Berman
2016-07-12 15:23             ` Eli Zaretskii
2016-07-12 20:13               ` Stephen Berman

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).