bug#28609: simple.el

bug#28609: simple.el

[Devon Sean McCullough]

Correct grammar; also, call a pair a pair.

--- a/simple.el	2017-09-26 05:31:16.000000000 -0400
+++ b/simple.el	2017-09-26 10:45:41.000000000 -0400
@@ -39,11 +39,11 @@
 
 (defcustom shell-command-dont-erase-buffer nil
   "If non-nil, output buffer is not erased between shell commands.
-Also, a non-nil value set the point in the output buffer
-once the command complete.
-The value `beg-last-out' set point at the beginning of the output,
-`end-last-out' set point at the end of the buffer, `save-point'
-restore the buffer position before the command."
+Also, a non-nil value sets the point in the output buffer
+once the command completes.
+The value `beg-last-out' sets point at the beginning of the output,
+`end-last-out' sets point at the end of the buffer, `save-point'
+restores the buffer position before the command."
   :type '(choice
           (const :tag "Erase buffer" nil)
           (const :tag "Set point to beginning of last output" beg-last-out)
@@ -53,9 +53,9 @@
   :version "26.1")
 
 (defvar shell-command-saved-pos nil
-  "Point position in the output buffer after command complete.
-It is an alist (BUFFER . POS), where BUFFER is the output
-buffer, and POS is the point position in BUFFER once the command finish.
+  "Point position in the output buffer after command completes.
+It is an alist of (BUFFER . POS), where BUFFER is the output
+buffer, and POS is the point position in BUFFER once the command finishes.
 This variable is used when `shell-command-dont-erase-buffer' is non-nil.")
 
 (defcustom idle-update-delay 0.5
@@ -1003,7 +1003,7 @@
 If METHOD is `delete-only', then delete the region; the return value
 is undefined.  If METHOD is nil, then return the content as a string.
 If METHOD is `bounds', then return the boundaries of the region
-as a list of the form (START . END).
+as a pair of (START . END) positions.
 If METHOD is anything else, delete the region and return its content
 as a string, after filtering it with `filter-buffer-substring', which
 is called with METHOD as its 3rd argument.")
@@ -5473,7 +5473,7 @@
        (progn (cl-assert (mark)) t)))
 
 (defun region-bounds ()
-  "Return the boundaries of the region as a list of (START . END) positions."
+  "Return the boundaries of the region as a pair of (START . END) positions."
   (funcall region-extract-function 'bounds))
 
 (defun region-noncontiguous-p ()

bug#28609: simple.el

[John Wiegley]

>>>>> "DSM" == Devon Sean McCullough <Emacs-Hacker2017@jovi.net> writes:

DSM> Correct grammar; also, call a pair a pair.

Thank you! Even minor corrections like these are quite helpful.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#28609: simple.el

[John Wiegley]

fixed 28609
thanks

This has been applied to the emacs-26 as a documentation correction, thank
you.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#28609: simple.el

[Johan Bockgård]

Devon Sean McCullough <Emacs-Hacker2017@jovi.net> writes:

> @@ -1003,7 +1003,7 @@
>  If METHOD is `delete-only', then delete the region; the return value
>  is undefined.  If METHOD is nil, then return the content as a string.
>  If METHOD is `bounds', then return the boundaries of the region
> -as a list of the form (START . END).
> +as a pair of (START . END) positions.

It's a list of pairs.

> @@ -5473,7 +5473,7 @@
>         (progn (cl-assert (mark)) t)))
>  
>  (defun region-bounds ()
> -  "Return the boundaries of the region as a list of (START . END) positions."
> +  "Return the boundaries of the region as a pair of (START . END) positions."
>    (funcall region-extract-function 'bounds))

Ditto.

bug#28609: simple.el

[John Wiegley]

>>>>> Johan Bockgård <bojohan@gnu.org> writes:

> It's a list of pairs.

Thanks, change applied in 98a37e60142340b9c2b4e6b17c373f4ae6a2d8b4 on emacs-26
branch, and 52a1da03b226b8686856259ac5d9474a8462322a on master.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#28609: simple.el

[Eli Zaretskii]

> From: Devon Sean McCullough <Emacs-Hacker2017@jovi.net>
> Date: Tue, 26 Sep 2017 10:51:04 -0400
> 
> Correct grammar; also, call a pair a pair.

Thanks for the fixes.

As for "pair", it's actually a cons cell, right?  Also, we do tend to
use the phrase "of the form ..." in our documentation, so there's no
need to remove that.

Finally, a nit: "Point position" is inherently ambiguous, so it's
better to use "Position of point" to disambiguate.  I made that change
in a followup commit.

bug#28609: simple.el

[Eli Zaretskii]

> From: Johan Bockgård <bojohan@gnu.org>
> Date: Tue, 26 Sep 2017 21:09:59 +0200
> Cc: John Wiegley <jwiegley@gmail.com>, 28609@debbugs.gnu.org
> 
> Devon Sean McCullough <Emacs-Hacker2017@jovi.net> writes:
> 
> > @@ -1003,7 +1003,7 @@
> >  If METHOD is `delete-only', then delete the region; the return value
> >  is undefined.  If METHOD is nil, then return the content as a string.
> >  If METHOD is `bounds', then return the boundaries of the region
> > -as a list of the form (START . END).
> > +as a pair of (START . END) positions.
> 
> It's a list of pairs.

Actually, a cons cell, right?

> > @@ -5473,7 +5473,7 @@
> >         (progn (cl-assert (mark)) t)))
> >  
> >  (defun region-bounds ()
> > -  "Return the boundaries of the region as a list of (START . END) positions."
> > +  "Return the boundaries of the region as a pair of (START . END) positions."
> >    (funcall region-extract-function 'bounds))
> 
> Ditto.

Ditto, right?

bug#28609: simple.el

[Drew Adams]

> Finally, a nit: "Point position" is inherently ambiguous, so it's
> better to use "Position of point" to disambiguate.  I made that change
> in a followup commit.

Apologies for not reading the thread.  Going only by what is written
here, which might not be fair/appropriate, "position of point" is
also ill-advised.  "Point" is the buffer position of the cursor
(text-insertion position).  "Position of point" would then mean
"position of position of the cursor".

Again, please ignore if interpreting this out of context is mistaken.

bug#28609: simple.el

[Eli Zaretskii]

> Date: Fri, 29 Sep 2017 19:30:40 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 28609@debbugs.gnu.org
> 
> > Finally, a nit: "Point position" is inherently ambiguous, so it's
> > better to use "Position of point" to disambiguate.  I made that change
> > in a followup commit.
> 
> Apologies for not reading the thread.  Going only by what is written
> here, which might not be fair/appropriate, "position of point" is
> also ill-advised.  "Point" is the buffer position of the cursor
> (text-insertion position).  "Position of point" would then mean
> "position of position of the cursor".

"Point" is Emacs terminology (and its definition in Glossary is not
"position of the cursor").  Since it's a widely-used term, I see no
need to replace "point" with its definition to judge that sentence's
grammar or style.  We use "position of point" in our manuals, and I
see no problem with that usage.

bug#28609: simple.el

[Johan Bockgård]

Eli Zaretskii <eliz@gnu.org> writes:

> Actually, a cons cell, right?

A list of cons cells (see region-noncontiguous-p).

bug#28609: simple.el

[Johan Bockgård]

Eli Zaretskii <eliz@gnu.org> writes:

> Actually, a cons cell, right?

No. Your latest doc fix is wrong; in all three cases
(shell-command-saved-pos, region-extract-function, region-bounds) the
value is a list of cons cells, not a single cons cell. (In the first
case it's an actual alist.)

bug#28609: simple.el

[Eli Zaretskii]

> From: Johan Bockgård <bojohan@gnu.org>
> Cc: jwiegley@gmail.com,  Emacs-Hacker2017@jovi.net,  28609@debbugs.gnu.org
> Date: Wed, 04 Oct 2017 22:24:27 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Actually, a cons cell, right?
> 
> No. Your latest doc fix is wrong; in all three cases
> (shell-command-saved-pos, region-extract-function, region-bounds) the
> value is a list of cons cells, not a single cons cell. (In the first
> case it's an actual alist.)

Thanks.  Can you propose a patch to fix these discrepancies?

bug#28609: simple.el

[Stefan Kangas]

close 28609 26.1
thanks

Johan Bockgård <bojohan@gnu.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Actually, a cons cell, right?
>
> No. Your latest doc fix is wrong; in all three cases
> (shell-command-saved-pos, region-extract-function, region-bounds) the
> value is a list of cons cells, not a single cons cell. (In the first
> case it's an actual alist.)

This was fixed here:

commit c9d71b31bf1144c3f11a82612f07a7bd649c25d4
Author: Eli Zaretskii <eliz@gnu.org>
Date:   Sat Oct 21 11:36:37 2017 +0300

    Fix doc strings in simple.el

    * lisp/simple.el (shell-command-saved-pos)
    (region-extract-function, region-bounds): Doc fixes.  (Bug#28609)

But the bug was left open, so I'm closing it now.

Best regards,
Stefan Kangas