bug#11262: help text fro sorted? predicate is wrong/misleading.

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Alexei Matveev]

Hi, Guilers,

I seem to have almost convinced at least one another person that
the docstring of sorted? predicate is at least misleading:

guile> (sorted? '(0 0) <)
#t

guile> (help sorted?)
`sorted?' is a primitive procedure in the (guile) module.

 -- Scheme Procedure: sorted? items less
     Return `#t' iff ITEMS is a list or a vector such that for all 1 <=
     i <= m, the predicate LESS returns true when applied to all
     elements i - 1 and i

My suggestion for the docstring is below (feel free to improve my broken
english).

Alexei

diff --git a/libguile/sort.c b/libguile/sort.c
index ecadd82..b71d101 100644
--- a/libguile/sort.c
+++ b/libguile/sort.c
@@ -101,9 +101,10 @@ SCM_DEFINE (scm_restricted_vector_sort_x,
"restricted-vector-sort!", 4, 0, 0,
  * (not (less? (list-ref list i) (list-ref list (- i 1)))). */
 SCM_DEFINE (scm_sorted_p, "sorted?", 2, 0, 0,
             (SCM items, SCM less),
-           "Return @code{#t} iff @var{items} is a list or a vector such that\n"
-           "for all 1 <= i <= m, the predicate @var{less} returns true when\n"
-           "applied to all elements i - 1 and i")
+           "Return @code{#t} iff @var{items} is  a list or a vector such\n"
+           "that for all  1 <= i <= m,  the predicate @var{less} returns\n"
+           "@code{#f} when comparing element i to the preceeding element\n"
+           "i - 1.")
 #define FUNC_NAME s_scm_sorted_p
 {
   long len, j;                 /* list/vector length, temp j */

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Noah Lavine]

It looks like you're right, and strangely enough there's a comment in
sort.c right above the definition of sorted? that has the correct
documentation.

I hope other people will correct me if I'm wrong, but I believe the
ideal thing for you to do now is submit a patch created with git
format-patch. That way you can write a git log entry, and your name
will be associated with the patch. You can see other recent log
entries for the standard guile format.

If you don't want to do this, I can do it for you, but I think it's
nicer if you are listed as the author since you did the work.

Thank you very much for reporting this,
Noah

On Tue, Apr 17, 2012 at 6:07 AM, Alexei Matveev
<alexei.matveev@gmail.com> wrote:
> Hi, Guilers,
>
> I seem to have almost convinced at least one another person that
> the docstring of sorted? predicate is at least misleading:
>
> guile> (sorted? '(0 0) <)
> #t
>
> guile> (help sorted?)
> `sorted?' is a primitive procedure in the (guile) module.
>
>  -- Scheme Procedure: sorted? items less
>     Return `#t' iff ITEMS is a list or a vector such that for all 1 <=
>     i <= m, the predicate LESS returns true when applied to all
>     elements i - 1 and i
>
> My suggestion for the docstring is below (feel free to improve my broken
> english).
>
> Alexei
>
> diff --git a/libguile/sort.c b/libguile/sort.c
> index ecadd82..b71d101 100644
> --- a/libguile/sort.c
> +++ b/libguile/sort.c
> @@ -101,9 +101,10 @@ SCM_DEFINE (scm_restricted_vector_sort_x,
> "restricted-vector-sort!", 4, 0, 0,
>  * (not (less? (list-ref list i) (list-ref list (- i 1)))). */
>  SCM_DEFINE (scm_sorted_p, "sorted?", 2, 0, 0,
>             (SCM items, SCM less),
> -           "Return @code{#t} iff @var{items} is a list or a vector such that\n"
> -           "for all 1 <= i <= m, the predicate @var{less} returns true when\n"
> -           "applied to all elements i - 1 and i")
> +           "Return @code{#t} iff @var{items} is  a list or a vector such\n"
> +           "that for all  1 <= i <= m,  the predicate @var{less} returns\n"
> +           "@code{#f} when comparing element i to the preceeding element\n"
> +           "i - 1.")
>  #define FUNC_NAME s_scm_sorted_p
>  {
>   long len, j;                 /* list/vector length, temp j */
>
>
>

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Alexei Matveev]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

Hi, Noah, Hi Guilers,

On Thu, Apr 19, 2012 at 3:14 AM, Noah Lavine <noah.b.lavine@gmail.com> wrote:
> It looks like you're right, and strangely enough there's a comment in
> sort.c right above the definition of sorted? that has the correct
> documentation.

Yes, that added me some confidence.

> I hope other people will correct me if I'm wrong, but I believe the
> ideal thing for you to do now is submit a patch created with git
> format-patch.

Attachment contains a patch created by "git format-patch origin".
The patch does exactly the same as the diff sent earlier.

Alexei

[-- Attachment #2: 0001-Reformulate-misleading-output-of-help-sorted.patch --]
[-- Type: application/octet-stream, Size: 1300 bytes --]

From 3ba571b35031aaff074d7d634e59a2a869fa2339 Mon Sep 17 00:00:00 2001
From: Alexei Matveev <matveev@theochem.tu-muenchen.de>
Date: Fri, 20 Apr 2012 12:06:52 +0200
Subject: [PATCH] Reformulate misleading output of (help sorted?).

Fixes <http://bugs.gnu.org/11262>.

* libguile/sort.c (scm_sorted_p): changed strings only
---
 libguile/sort.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/libguile/sort.c b/libguile/sort.c
index ecadd82..b71d101 100644
--- a/libguile/sort.c
+++ b/libguile/sort.c
@@ -101,9 +101,10 @@ SCM_DEFINE (scm_restricted_vector_sort_x, "restricted-vector-sort!", 4, 0, 0,
  * (not (less? (list-ref list i) (list-ref list (- i 1)))). */
 SCM_DEFINE (scm_sorted_p, "sorted?", 2, 0, 0,
             (SCM items, SCM less),
-	    "Return @code{#t} iff @var{items} is a list or a vector such that\n"
-	    "for all 1 <= i <= m, the predicate @var{less} returns true when\n"
-	    "applied to all elements i - 1 and i")
+	    "Return @code{#t} iff @var{items} is  a list or a vector such\n"
+	    "that for all  1 <= i <= m,  the predicate @var{less} returns\n"
+	    "@code{#f} when comparing element i to the preceeding element\n"
+	    "i - 1.")
 #define FUNC_NAME s_scm_sorted_p
 {
   long len, j;			/* list/vector length, temp j */
-- 
1.5.6.5

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Ludovic Courtès]

Hi Alexei,

(Removing Cc: guile-devel.)

Thanks for the patch!

Alexei Matveev <alexei.matveev@gmail.com> skribis:

> --- a/libguile/sort.c
> +++ b/libguile/sort.c
> @@ -101,9 +101,10 @@ SCM_DEFINE (scm_restricted_vector_sort_x, "restricted-vector-sort!", 4, 0, 0,
>   * (not (less? (list-ref list i) (list-ref list (- i 1)))). */
>  SCM_DEFINE (scm_sorted_p, "sorted?", 2, 0, 0,
>              (SCM items, SCM less),
> -	    "Return @code{#t} iff @var{items} is a list or a vector such that\n"
> -	    "for all 1 <= i <= m, the predicate @var{less} returns true when\n"
> -	    "applied to all elements i - 1 and i")
> +	    "Return @code{#t} iff @var{items} is  a list or a vector such\n"
> +	    "that for all  1 <= i <= m,  the predicate @var{less} returns\n"
> +	    "@code{#f} when comparing element i to the preceeding element\n"
> +	    "i - 1.")

How about this:

  Return @code{#t} iff @var{items} is a list or vector such that,
  for each element @var{x} and the next element @var{y} of @var{items},
  @code{(@var{less} @var{x} @var{y})} returns @code{#f}.

This avoids use of ‘m’, which would need to be defined, and makes it
clear what “comparing element i to the preceeding element” means.

What do you think?

Thanks,
Ludo’.

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Ian Hulin]

Hi Ludo, Alexei,
Please avoid C-speak in documentation if possible.
(See below)

On 22/04/12 00:38, Ludovic Courtès wrote:
> Hi Alexei,
> 
> (Removing Cc: guile-devel.)
> 
> Thanks for the patch!
> 
> Alexei Matveev <alexei.matveev@gmail.com> skribis:
> 
>> --- a/libguile/sort.c +++ b/libguile/sort.c @@ -101,9 +101,10 @@
>> SCM_DEFINE (scm_restricted_vector_sort_x,
>> "restricted-vector-sort!", 4, 0, 0, * (not (less? (list-ref list
>> i) (list-ref list (- i 1)))). */ SCM_DEFINE (scm_sorted_p,
>> "sorted?", 2, 0, 0, (SCM items, SCM less), -	    "Return
>> @code{#t} iff @var{items} is a list or a vector such that\n" -
>> "for all 1 <= i <= m, the predicate @var{less} returns true
>> when\n" -	    "applied to all elements i - 1 and i") +
>> "Return @code{#t} iff @var{items} is  a list or a vector such\n" 
>> +	    "that for all  1 <= i <= m,  the predicate @var{less}
>> returns\n" +	    "@code{#f} when comparing element i to the
>> preceeding element\n" +	    "i - 1.")
> 
> How about this:
> 
> Return @code{#t} iff @var{items} is a list or vector such that, for
> each element @var{x} and the next element @var{y} of @var{items}, 
> @code{(@var{less} @var{x} @var{y})} returns @code{#f}.
> 
> This avoids use of ‘m’, which would need to be defined, and makes
> it clear what “comparing element i to the preceeding element”
> means.
> 
> What do you think?
> 

Please spell out iff, which I presume means "if-and-only-if".  The
reason is that on first reading it looks ambiguous to old dinosaurs
like me who may have experience of programming languages using iff as
a token for "if-false" back in the Jurassic period.

Or maybe say:

Return @code{#t} when @var{items} is a list or vector such that,
   for each element @var{x} and the next element @var{y} of @var{items},
   @code{(@var{less} @var{x} @var{y})} returns @code{#f}.


Cheers,
Ian Hulin

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Ludovic Courtès]

Hi,

Ian Hulin <ian@hulin.org.uk> skribis:

> Please spell out iff, which I presume means "if-and-only-if".

I think it’s fine: it’s used consistently in the manual, and it’s
documented (info "(guile) Typographical Conventions").

Ludo’.

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Alexei Matveev]

>> How about this:
>>
>> Return @code{#t} iff @var{items} is a list or vector such that, for
>> each element @var{x} and the next element @var{y} of @var{items},
>> @code{(@var{less} @var{x} @var{y})} returns @code{#f}.
>>
>> This avoids use of ‘m’, which would need to be defined, and makes
>> it clear what “comparing element i to the preceeding element”
>> means.

Yes, "m" should not be mentioned. But I think you got it backwards again:

Return @code{#t} iff @var{items} is a list or vector such that, for
each element @var{x} and the next element @var{y} of @var{items},
@code{(@var{less} @var{y} @var{x})} returns @code{#f}.

Note the order of x and y in (less? y x) in the last line.

Alexei

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Ludovic Courtès]

Hi Alexei,

Alexei Matveev <alexei.matveev@gmail.com> skribis:

>>> How about this:
>>>
>>> Return @code{#t} iff @var{items} is a list or vector such that, for
>>> each element @var{x} and the next element @var{y} of @var{items},
>>> @code{(@var{less} @var{x} @var{y})} returns @code{#f}.
>>>
>>> This avoids use of ‘m’, which would need to be defined, and makes
>>> it clear what “comparing element i to the preceeding element”
>>> means.
>
> Yes, "m" should not be mentioned. But I think you got it backwards again:
>
> Return @code{#t} iff @var{items} is a list or vector such that, for
> each element @var{x} and the next element @var{y} of @var{items},
> @code{(@var{less} @var{y} @var{x})} returns @code{#f}.
>
> Note the order of x and y in (less? y x) in the last line.

Oops, indeed.

Pushed in commit fd07759b7d4c9d631090b04855ab81b6a2109e9e.

Thanks for the report, patch, and review!

Ludo’.

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Marijn]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 22-04-12 14:53, Ludovic Courtès wrote:
> Hi Alexei,
> 
> Alexei Matveev <alexei.matveev@gmail.com> skribis:
> 
>>>> How about this:
>>>> 
>>>> Return @code{#t} iff @var{items} is a list or vector such
>>>> that, for each element @var{x} and the next element @var{y}
>>>> of @var{items}, @code{(@var{less} @var{x} @var{y})} returns
>>>> @code{#f}.
>>>> 
>>>> This avoids use of ‘m’, which would need to be defined, and
>>>> makes it clear what “comparing element i to the preceeding
>>>> element” means.
>> 
>> Yes, "m" should not be mentioned. But I think you got it
>> backwards again:
>> 
>> Return @code{#t} iff @var{items} is a list or vector such that,
>> for each element @var{x} and the next element @var{y} of
>> @var{items}, @code{(@var{less} @var{y} @var{x})} returns
>> @code{#f}.
>> 
>> Note the order of x and y in (less? y x) in the last line.
> 
> Oops, indeed.
> 
> Pushed in commit fd07759b7d4c9d631090b04855ab81b6a2109e9e.
> 
> Thanks for the report, patch, and review!

So it seems the rewording was about whether a list like '(2 2) is
sorted or not. IIUC the rewording makes it a sorted list according to
<, because (not (< 2 2)) is #f.

Is that really what is wanted?

Marijn
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk+Y+b0ACgkQp/VmCx0OL2zB1wCfZ9Zp5JpJw4frqfpXOnCp2xvM
75kAnjJi/CpmNvh7M9sqQTbFzj7pTniN
=6xtZ
-----END PGP SIGNATURE-----

bug#11262: help text fro sorted? predicate is wrong/misleading.

[Noah Lavine]

Well, I wanted to reply to you by seeing how sorted? was used. But
looking at the Guile source code, it appears that there are no uses of
sorted? at all! However, I think it needs to stay like it is, because
other code may depend on it.

Noah

On Thu, Apr 26, 2012 at 3:31 AM, Marijn <hkBst@gentoo.org> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 22-04-12 14:53, Ludovic Courtès wrote:
>> Hi Alexei,
>>
>> Alexei Matveev <alexei.matveev@gmail.com> skribis:
>>
>>>>> How about this:
>>>>>
>>>>> Return @code{#t} iff @var{items} is a list or vector such
>>>>> that, for each element @var{x} and the next element @var{y}
>>>>> of @var{items}, @code{(@var{less} @var{x} @var{y})} returns
>>>>> @code{#f}.
>>>>>
>>>>> This avoids use of ‘m’, which would need to be defined, and
>>>>> makes it clear what “comparing element i to the preceeding
>>>>> element” means.
>>>
>>> Yes, "m" should not be mentioned. But I think you got it
>>> backwards again:
>>>
>>> Return @code{#t} iff @var{items} is a list or vector such that,
>>> for each element @var{x} and the next element @var{y} of
>>> @var{items}, @code{(@var{less} @var{y} @var{x})} returns
>>> @code{#f}.
>>>
>>> Note the order of x and y in (less? y x) in the last line.
>>
>> Oops, indeed.
>>
>> Pushed in commit fd07759b7d4c9d631090b04855ab81b6a2109e9e.
>>
>> Thanks for the report, patch, and review!
>
> So it seems the rewording was about whether a list like '(2 2) is
> sorted or not. IIUC the rewording makes it a sorted list according to
> <, because (not (< 2 2)) is #f.
>
> Is that really what is wanted?
>
> Marijn
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.19 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAk+Y+b0ACgkQp/VmCx0OL2zB1wCfZ9Zp5JpJw4frqfpXOnCp2xvM
> 75kAnjJi/CpmNvh7M9sqQTbFzj7pTniN
> =6xtZ
> -----END PGP SIGNATURE-----
>
>
>