unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#35916: [PATCH] checkdoc fixes in bookmark.el
@ 2019-05-26 10:59 Stefan Kangas
  2019-05-27  3:52 ` Drew Adams
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Kangas @ 2019-05-26 10:59 UTC (permalink / raw)
  To: 35916


[-- Attachment #1.1: Type: text/plain, Size: 110 bytes --]

I ran checkdoc on bookmark.el and fixed some of the errors.
Please see attached patch.

Thanks,
Stefan Kangas

[-- Attachment #1.2: Type: text/html, Size: 209 bytes --]

[-- Attachment #2: checkdoc-fixes-in-bookmark.el.patch --]
[-- Type: text/x-patch, Size: 6332 bytes --]

From d46eef6146310c700cd04f499119c164f5461391 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sun, 26 May 2019 12:48:26 +0200
Subject: [PATCH] Checkdoc fixes in bookmark.el

* bookmark.el (bookmark-bmenu-inline-header-height)
(bookmark-bmenu-marks-width, bookmark-map, bookmark-alist)
(bookmark-quit-flag, bookmark-name-from-full-record)
(bookmark-set-internal, bookmark-insert-annotation)
(bookmark--jump-via, bookmark-bmenu-set-header)
(bookmark-show-annotation, bookmark-bmenu-other-window-with-mouse)
(bookmark-bmenu-relocate): Checkdoc docstring fixes.
---
 lisp/bookmark.el | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index b1fe690dac..ef03023965 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -136,12 +136,13 @@ bookmark-bmenu-use-header-line
   :group 'bookmark)
 
 (defconst bookmark-bmenu-inline-header-height 2
-  "Number of lines used for the *Bookmark List* header
-\(only significant when `bookmark-bmenu-use-header-line' is nil).")
+  "Number of lines used for the *Bookmark List* header.
+\(This is only significant when `bookmark-bmenu-use-header-line'
+is nil.)")
 
 (defconst bookmark-bmenu-marks-width 2
-  "Number of columns (chars) used for the *Bookmark List* marks column,
-including the annotations column.")
+  "Number of columns (chars) used for the *Bookmark List* marks column.
+This includes the annotations column.")
 
 (defcustom bookmark-bmenu-file-column 30
   "Column at which to display filenames in a buffer listing bookmarks.
@@ -221,7 +222,7 @@ bookmark-map
   "Keymap containing bindings to bookmark functions.
 It is not bound to any key by default: to bind it
 so that you have a bookmark prefix, just use `global-set-key' and bind a
-key of your choice to `bookmark-map'.  All interactive bookmark
+key of your choice to variable `bookmark-map'.  All interactive bookmark
 functions have a binding in this keymap.")
 
 ;;;###autoload (fset 'bookmark-map bookmark-map)
@@ -260,7 +261,7 @@ bookmark-alist
  ANNOTATION is a string that describes the bookmark.
    See options `bookmark-use-annotations' and
    `bookmark-automatically-show-annotations'.
- HANDLER is a function that provides the bookmark-jump behavior for a
+ HANDLER is a function that provides the `bookmark-jump' behavior for a
  specific kind of bookmark.  This is the case for Info bookmarks,
  for instance.  HANDLER must accept a bookmark as its single argument.")
 
@@ -301,7 +302,7 @@ bookmark-yank-point
 
 
 (defvar bookmark-quit-flag nil
-  "Non nil make `bookmark-bmenu-search' quit immediately.")
+  "Non-nil means `bookmark-bmenu-search' quits immediately.")
 \f
 ;; Helper functions and macros.
 
@@ -318,8 +319,8 @@ with-buffer-modified-unmodified
 ;; Everyone else should go through them.
 
 (defun bookmark-name-from-full-record (bookmark-record)
-  "Return the name of BOOKMARK-RECORD.  BOOKMARK-RECORD is, e.g.,
-one element from `bookmark-alist'."
+  "Return the name of BOOKMARK-RECORD.
+BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'."
   (car bookmark-record))
 
 
@@ -811,7 +812,7 @@ bookmark-set-internal
            (cond
             ((eq overwrite-or-push nil)
              (if (bookmark-get-bookmark str t)
-                 (error "A bookmark named \"%s\" already exists." str)
+                 (error "A bookmark named \"%s\" already exists" str)
                (bookmark-store str (cdr record) nil)))
             ((eq overwrite-or-push 'overwrite)
              (bookmark-store str (cdr record) nil))
@@ -936,6 +937,7 @@ bookmark-edit-annotation-mode-map
   "Keymap for editing an annotation of a bookmark.")
 
 (defun bookmark-insert-annotation (bookmark-name-or-record)
+  "Insert annotation for BOOKMARK-NAME-OR-RECORD at point."
   (insert (funcall bookmark-edit-annotation-text-func bookmark-name-or-record))
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (if (and annotation (not (string-equal annotation "")))
@@ -1065,8 +1067,8 @@ bookmark-after-jump-hook
 Useful for example to unhide text in `outline-mode'.")
 
 (defun bookmark--jump-via (bookmark-name-or-record display-function)
-  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION with
-current buffer as argument.
+  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION.
+DISPLAY-FUNCTION is called with the current buffer as argument.
 
 After calling DISPLAY-FUNCTION, set window point to the point specified
 by BOOKMARK-NAME-OR-RECORD, if necessary, run `bookmark-after-jump-hook',
@@ -1675,7 +1677,7 @@ 'list-bookmarks
 (defalias 'edit-bookmarks 'bookmark-bmenu-list)
 
 (defun bookmark-bmenu-set-header ()
-  "Sets the immutable header line."
+  "Set the immutable header line."
   (let ((header (concat "%% " "Bookmark")))
     (when bookmark-bmenu-toggle-filenames
       (setq header (concat header
@@ -1835,8 +1837,8 @@ bookmark-bmenu-bookmark
 
 
 (defun bookmark-show-annotation (bookmark-name-or-record)
-  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer,
-if an annotation exists."
+  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer.
+If the annotation does not exists, do nothing."
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (when (and annotation (not (string-equal annotation "")))
       (save-excursion
@@ -1997,7 +1999,9 @@ bookmark-bmenu-switch-other-window
     (bookmark--jump-via bookmark fun)))
 
 (defun bookmark-bmenu-other-window-with-mouse (event)
-  "Select bookmark at the mouse pointer in other window, leaving bookmark menu visible."
+  "Select bookmark at the mouse pointer in other window.
+Move point to the position of EVENT, and leave bookmark menu
+visible."
   (interactive "e")
   (with-current-buffer (window-buffer (posn-window (event-end event)))
     (save-excursion
@@ -2123,8 +2127,8 @@ bookmark-bmenu-locate
     (message "%s" (bookmark-location bmrk))))
 
 (defun bookmark-bmenu-relocate ()
-  "Change the file path of the bookmark on the current line,
-  prompting with completion for the new path."
+  "Change the file path of the bookmark on the current line.
+Prompt with completion for the new path."
   (interactive)
   (let ((bmrk (bookmark-bmenu-bookmark))
         (thispoint (point)))
-- 
2.11.0


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

* bug#35916: [PATCH] checkdoc fixes in bookmark.el
  2019-05-26 10:59 bug#35916: [PATCH] checkdoc fixes in bookmark.el Stefan Kangas
@ 2019-05-27  3:52 ` Drew Adams
  2019-05-27 21:36   ` Stefan Kangas
  0 siblings, 1 reply; 7+ messages in thread
From: Drew Adams @ 2019-05-27  3:52 UTC (permalink / raw)
  To: Stefan Kangas, 35916

(Minor feedback.) 

1. This one is wrong, IMO:

- HANDLER is a function that provides the bookmark-jump behavior for a
+ HANDLER is a function that provides the `bookmark-jump' behavior for a

That use of "bookmark-jump" is just an adjective, not the name of the specific command `bookmark-jump'.

The point of the handler is to provide alternative behavior from what command `bookmark-jump' provides.  But in all cases that alternative is some kind of a "bookmark-jump behavior" - not behavior of command `bookmark-jump' but some bookmark-jumping behavior.

You could in fact alternatively say "bookmark-jumping behavior".  But there's no need to (and it's not about a bookmark jumping but about jumping to a bookmark location).

2. This one has a typo:

+If the annotation does not exists, do nothing."
                            ^
                            exist

3. This one makes some helpful corrections, but Emacs doesn't
call this a "file path".  It calls it an "absolute file name".

+  "Change the file path of the bookmark on the current line.
+Prompt with completion for the new path."


All the other changes look like improvements, to me.  Thx.

(And bug reports #35917 and #35918 are also welcome.)





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

* bug#35916: [PATCH] checkdoc fixes in bookmark.el
  2019-05-27  3:52 ` Drew Adams
@ 2019-05-27 21:36   ` Stefan Kangas
  2019-05-30 17:53     ` Stefan Kangas
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Kangas @ 2019-05-27 21:36 UTC (permalink / raw)
  To: Drew Adams; +Cc: 35916

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

Drew Adams <drew.adams@oracle.com> writes:
> (Minor feedback.)

Fixed all your comments in the attached patch.

> All the other changes look like improvements, to me.  Thx.

Thank you for providing this feedback.

Best regards,
Stefan Kangas

[-- Attachment #2: 0001-Checkdoc-fixes-in-bookmark.el.patch --]
[-- Type: text/x-patch, Size: 5851 bytes --]

From 5deabc350a553a41e379399335aeeacb8bb6628b Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sun, 26 May 2019 12:48:26 +0200
Subject: [PATCH] Checkdoc fixes in bookmark.el

* bookmark.el (bookmark-bmenu-inline-header-height)
(bookmark-bmenu-marks-width, bookmark-map, bookmark-quit-flag)
(bookmark-name-from-full-record, bookmark-set-internal)
(bookmark-insert-annotation, bookmark--jump-via)
(bookmark-bmenu-set-header, bookmark-show-annotation)
(bookmark-bmenu-other-window-with-mouse, bookmark-bmenu-relocate):
Checkdoc docstring fixes.
---
 lisp/bookmark.el | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index b1fe690dac..c3899bcfc0 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -136,12 +136,13 @@ bookmark-bmenu-use-header-line
   :group 'bookmark)
 
 (defconst bookmark-bmenu-inline-header-height 2
-  "Number of lines used for the *Bookmark List* header
-\(only significant when `bookmark-bmenu-use-header-line' is nil).")
+  "Number of lines used for the *Bookmark List* header.
+\(This is only significant when `bookmark-bmenu-use-header-line'
+is nil.)")
 
 (defconst bookmark-bmenu-marks-width 2
-  "Number of columns (chars) used for the *Bookmark List* marks column,
-including the annotations column.")
+  "Number of columns (chars) used for the *Bookmark List* marks column.
+This includes the annotations column.")
 
 (defcustom bookmark-bmenu-file-column 30
   "Column at which to display filenames in a buffer listing bookmarks.
@@ -221,7 +222,7 @@ bookmark-map
   "Keymap containing bindings to bookmark functions.
 It is not bound to any key by default: to bind it
 so that you have a bookmark prefix, just use `global-set-key' and bind a
-key of your choice to `bookmark-map'.  All interactive bookmark
+key of your choice to variable `bookmark-map'.  All interactive bookmark
 functions have a binding in this keymap.")
 
 ;;;###autoload (fset 'bookmark-map bookmark-map)
@@ -301,7 +302,7 @@ bookmark-yank-point
 
 
 (defvar bookmark-quit-flag nil
-  "Non nil make `bookmark-bmenu-search' quit immediately.")
+  "Non-nil means `bookmark-bmenu-search' quits immediately.")
 \f
 ;; Helper functions and macros.
 
@@ -318,8 +319,8 @@ with-buffer-modified-unmodified
 ;; Everyone else should go through them.
 
 (defun bookmark-name-from-full-record (bookmark-record)
-  "Return the name of BOOKMARK-RECORD.  BOOKMARK-RECORD is, e.g.,
-one element from `bookmark-alist'."
+  "Return the name of BOOKMARK-RECORD.
+BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'."
   (car bookmark-record))
 
 
@@ -811,7 +812,7 @@ bookmark-set-internal
            (cond
             ((eq overwrite-or-push nil)
              (if (bookmark-get-bookmark str t)
-                 (error "A bookmark named \"%s\" already exists." str)
+                 (error "A bookmark named \"%s\" already exists" str)
                (bookmark-store str (cdr record) nil)))
             ((eq overwrite-or-push 'overwrite)
              (bookmark-store str (cdr record) nil))
@@ -936,6 +937,7 @@ bookmark-edit-annotation-mode-map
   "Keymap for editing an annotation of a bookmark.")
 
 (defun bookmark-insert-annotation (bookmark-name-or-record)
+  "Insert annotation for BOOKMARK-NAME-OR-RECORD at point."
   (insert (funcall bookmark-edit-annotation-text-func bookmark-name-or-record))
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (if (and annotation (not (string-equal annotation "")))
@@ -1065,8 +1067,8 @@ bookmark-after-jump-hook
 Useful for example to unhide text in `outline-mode'.")
 
 (defun bookmark--jump-via (bookmark-name-or-record display-function)
-  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION with
-current buffer as argument.
+  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION.
+DISPLAY-FUNCTION is called with the current buffer as argument.
 
 After calling DISPLAY-FUNCTION, set window point to the point specified
 by BOOKMARK-NAME-OR-RECORD, if necessary, run `bookmark-after-jump-hook',
@@ -1675,7 +1677,7 @@ 'list-bookmarks
 (defalias 'edit-bookmarks 'bookmark-bmenu-list)
 
 (defun bookmark-bmenu-set-header ()
-  "Sets the immutable header line."
+  "Set the immutable header line."
   (let ((header (concat "%% " "Bookmark")))
     (when bookmark-bmenu-toggle-filenames
       (setq header (concat header
@@ -1835,8 +1837,8 @@ bookmark-bmenu-bookmark
 
 
 (defun bookmark-show-annotation (bookmark-name-or-record)
-  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer,
-if an annotation exists."
+  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer.
+If the annotation does not exist, do nothing."
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (when (and annotation (not (string-equal annotation "")))
       (save-excursion
@@ -1997,7 +1999,9 @@ bookmark-bmenu-switch-other-window
     (bookmark--jump-via bookmark fun)))
 
 (defun bookmark-bmenu-other-window-with-mouse (event)
-  "Select bookmark at the mouse pointer in other window, leaving bookmark menu visible."
+  "Select bookmark at the mouse pointer in other window.
+Move point to the position of EVENT, and leave bookmark menu
+visible."
   (interactive "e")
   (with-current-buffer (window-buffer (posn-window (event-end event)))
     (save-excursion
@@ -2123,8 +2127,8 @@ bookmark-bmenu-locate
     (message "%s" (bookmark-location bmrk))))
 
 (defun bookmark-bmenu-relocate ()
-  "Change the file path of the bookmark on the current line,
-  prompting with completion for the new path."
+  "Change the absolute file name of the bookmark on the current line.
+Prompt with completion for the new path."
   (interactive)
   (let ((bmrk (bookmark-bmenu-bookmark))
         (thispoint (point)))
-- 
2.11.0


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

* bug#35916: [PATCH] checkdoc fixes in bookmark.el
  2019-05-27 21:36   ` Stefan Kangas
@ 2019-05-30 17:53     ` Stefan Kangas
  2019-06-08  8:37       ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Kangas @ 2019-05-30 17:53 UTC (permalink / raw)
  Cc: 35916

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

Please find attached an updated version of the patch with an improved commit
message.

Thanks,
Stefan Kangas

[-- Attachment #2: 0001-Checkdoc-fixes-in-lisp-bookmark.el.patch --]
[-- Type: text/x-patch, Size: 5874 bytes --]

From dac22dd0b713844743a361b6245973b2e38bd783 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sun, 26 May 2019 12:48:26 +0200
Subject: [PATCH] Checkdoc fixes in lisp/bookmark.el

* lisp/bookmark.el (bookmark-bmenu-inline-header-height)
(bookmark-bmenu-marks-width, bookmark-map, bookmark-quit-flag)
(bookmark-name-from-full-record, bookmark-set-internal)
(bookmark-insert-annotation, bookmark--jump-via)
(bookmark-bmenu-set-header, bookmark-show-annotation)
(bookmark-bmenu-other-window-with-mouse, bookmark-bmenu-relocate):
Checkdoc docstring fixes.  (Bug#35916)
---
 lisp/bookmark.el | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index b1fe690dac..c3899bcfc0 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -136,12 +136,13 @@ bookmark-bmenu-use-header-line
   :group 'bookmark)
 
 (defconst bookmark-bmenu-inline-header-height 2
-  "Number of lines used for the *Bookmark List* header
-\(only significant when `bookmark-bmenu-use-header-line' is nil).")
+  "Number of lines used for the *Bookmark List* header.
+\(This is only significant when `bookmark-bmenu-use-header-line'
+is nil.)")
 
 (defconst bookmark-bmenu-marks-width 2
-  "Number of columns (chars) used for the *Bookmark List* marks column,
-including the annotations column.")
+  "Number of columns (chars) used for the *Bookmark List* marks column.
+This includes the annotations column.")
 
 (defcustom bookmark-bmenu-file-column 30
   "Column at which to display filenames in a buffer listing bookmarks.
@@ -221,7 +222,7 @@ bookmark-map
   "Keymap containing bindings to bookmark functions.
 It is not bound to any key by default: to bind it
 so that you have a bookmark prefix, just use `global-set-key' and bind a
-key of your choice to `bookmark-map'.  All interactive bookmark
+key of your choice to variable `bookmark-map'.  All interactive bookmark
 functions have a binding in this keymap.")
 
 ;;;###autoload (fset 'bookmark-map bookmark-map)
@@ -301,7 +302,7 @@ bookmark-yank-point
 
 
 (defvar bookmark-quit-flag nil
-  "Non nil make `bookmark-bmenu-search' quit immediately.")
+  "Non-nil means `bookmark-bmenu-search' quits immediately.")
 \f
 ;; Helper functions and macros.
 
@@ -318,8 +319,8 @@ with-buffer-modified-unmodified
 ;; Everyone else should go through them.
 
 (defun bookmark-name-from-full-record (bookmark-record)
-  "Return the name of BOOKMARK-RECORD.  BOOKMARK-RECORD is, e.g.,
-one element from `bookmark-alist'."
+  "Return the name of BOOKMARK-RECORD.
+BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'."
   (car bookmark-record))
 
 
@@ -811,7 +812,7 @@ bookmark-set-internal
            (cond
             ((eq overwrite-or-push nil)
              (if (bookmark-get-bookmark str t)
-                 (error "A bookmark named \"%s\" already exists." str)
+                 (error "A bookmark named \"%s\" already exists" str)
                (bookmark-store str (cdr record) nil)))
             ((eq overwrite-or-push 'overwrite)
              (bookmark-store str (cdr record) nil))
@@ -936,6 +937,7 @@ bookmark-edit-annotation-mode-map
   "Keymap for editing an annotation of a bookmark.")
 
 (defun bookmark-insert-annotation (bookmark-name-or-record)
+  "Insert annotation for BOOKMARK-NAME-OR-RECORD at point."
   (insert (funcall bookmark-edit-annotation-text-func bookmark-name-or-record))
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (if (and annotation (not (string-equal annotation "")))
@@ -1065,8 +1067,8 @@ bookmark-after-jump-hook
 Useful for example to unhide text in `outline-mode'.")
 
 (defun bookmark--jump-via (bookmark-name-or-record display-function)
-  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION with
-current buffer as argument.
+  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION.
+DISPLAY-FUNCTION is called with the current buffer as argument.
 
 After calling DISPLAY-FUNCTION, set window point to the point specified
 by BOOKMARK-NAME-OR-RECORD, if necessary, run `bookmark-after-jump-hook',
@@ -1675,7 +1677,7 @@ 'list-bookmarks
 (defalias 'edit-bookmarks 'bookmark-bmenu-list)
 
 (defun bookmark-bmenu-set-header ()
-  "Sets the immutable header line."
+  "Set the immutable header line."
   (let ((header (concat "%% " "Bookmark")))
     (when bookmark-bmenu-toggle-filenames
       (setq header (concat header
@@ -1835,8 +1837,8 @@ bookmark-bmenu-bookmark
 
 
 (defun bookmark-show-annotation (bookmark-name-or-record)
-  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer,
-if an annotation exists."
+  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer.
+If the annotation does not exist, do nothing."
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (when (and annotation (not (string-equal annotation "")))
       (save-excursion
@@ -1997,7 +1999,9 @@ bookmark-bmenu-switch-other-window
     (bookmark--jump-via bookmark fun)))
 
 (defun bookmark-bmenu-other-window-with-mouse (event)
-  "Select bookmark at the mouse pointer in other window, leaving bookmark menu visible."
+  "Select bookmark at the mouse pointer in other window.
+Move point to the position of EVENT, and leave bookmark menu
+visible."
   (interactive "e")
   (with-current-buffer (window-buffer (posn-window (event-end event)))
     (save-excursion
@@ -2123,8 +2127,8 @@ bookmark-bmenu-locate
     (message "%s" (bookmark-location bmrk))))
 
 (defun bookmark-bmenu-relocate ()
-  "Change the file path of the bookmark on the current line,
-  prompting with completion for the new path."
+  "Change the absolute file name of the bookmark on the current line.
+Prompt with completion for the new path."
   (interactive)
   (let ((bmrk (bookmark-bmenu-bookmark))
         (thispoint (point)))
-- 
2.11.0


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

* bug#35916: [PATCH] checkdoc fixes in bookmark.el
  2019-05-30 17:53     ` Stefan Kangas
@ 2019-06-08  8:37       ` Eli Zaretskii
  2019-06-08 17:22         ` Stefan Kangas
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2019-06-08  8:37 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 35916

> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 30 May 2019 19:53:58 +0200
> Cc: 35916@debbugs.gnu.org
> 
> Please find attached an updated version of the patch with an improved commit
> message.

Thanks, this is OK, with one comment:

>  (defun bookmark-bmenu-other-window-with-mouse (event)
> -  "Select bookmark at the mouse pointer in other window, leaving bookmark menu visible."
> +  "Select bookmark at the mouse pointer in other window.
> +Move point to the position of EVENT, and leave bookmark menu
> +visible."

The first line of the doc string should mention the arguments, in this
case EVENT.  Could you please rework this hunk and resubmit the patch?





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

* bug#35916: [PATCH] checkdoc fixes in bookmark.el
  2019-06-08  8:37       ` Eli Zaretskii
@ 2019-06-08 17:22         ` Stefan Kangas
  2019-06-09  6:51           ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Kangas @ 2019-06-08 17:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 35916

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

Eli Zaretskii <eliz@gnu.org> writes:
> The first line of the doc string should mention the arguments, in this
> case EVENT.  Could you please rework this hunk and resubmit the patch?

Sure.  I've attached a patch with the following fix:

 (defun bookmark-bmenu-other-window-with-mouse (event)
-  "Select bookmark at the mouse pointer in other window, leaving
bookmark menu visible."
+  "Jump to bookmark at mouse EVENT position in other window.
+Move point in menu buffer to the position of EVENT and leave
+bookmark menu visible."

[Gmail might add some line breaks here, sorry about that...]

I also included the following additional fix:

 (defun bookmark-get-bookmark-record (bookmark-name-or-record)
-  "Return the record portion of the entry for BOOKMARK-NAME-OR-RECORD in
-`bookmark-alist' (that is, all information but the name)."
+  "Return the record portion of BOOKMARK-NAME-OR-RECORD in `bookmark-alist'.
+In other words, return all information but the name."

Thanks,
Stefan Kangas

[-- Attachment #2: 0001-Checkdoc-fixes-in-lisp-bookmark.el-4.patch --]
[-- Type: application/octet-stream, Size: 6474 bytes --]

From a1c5c199b5387f80f7fd1c5cf575eb1e1f7c5b73 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sun, 26 May 2019 12:48:26 +0200
Subject: [PATCH] Checkdoc fixes in lisp/bookmark.el

* lisp/bookmark.el (bookmark-bmenu-inline-header-height)
(bookmark-bmenu-marks-width, bookmark-map, bookmark-quit-flag)
(bookmark-name-from-full-record, bookmark-set-internal)
(bookmark-insert-annotation, bookmark--jump-via)
(bookmark-bmenu-set-header, bookmark-show-annotation)
(bookmark-bmenu-other-window-with-mouse, bookmark-bmenu-relocate):
Checkdoc docstring fixes.  (Bug#35916)
---
 lisp/bookmark.el | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index ed71dd1ade..dd8470168c 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -136,12 +136,13 @@ bookmark-bmenu-use-header-line
   :group 'bookmark)
 
 (defconst bookmark-bmenu-inline-header-height 2
-  "Number of lines used for the *Bookmark List* header
-\(only significant when `bookmark-bmenu-use-header-line' is nil).")
+  "Number of lines used for the *Bookmark List* header.
+\(This is only significant when `bookmark-bmenu-use-header-line'
+is nil.)")
 
 (defconst bookmark-bmenu-marks-width 2
-  "Number of columns (chars) used for the *Bookmark List* marks column,
-including the annotations column.")
+  "Number of columns (chars) used for the *Bookmark List* marks column.
+This includes the annotations column.")
 
 (defcustom bookmark-bmenu-file-column 30
   "Column at which to display filenames in a buffer listing bookmarks.
@@ -221,7 +222,7 @@ bookmark-map
   "Keymap containing bindings to bookmark functions.
 It is not bound to any key by default: to bind it
 so that you have a bookmark prefix, just use `global-set-key' and bind a
-key of your choice to `bookmark-map'.  All interactive bookmark
+key of your choice to variable `bookmark-map'.  All interactive bookmark
 functions have a binding in this keymap.")
 
 ;;;###autoload (fset 'bookmark-map bookmark-map)
@@ -301,7 +302,7 @@ bookmark-yank-point
 
 
 (defvar bookmark-quit-flag nil
-  "Non nil make `bookmark-bmenu-search' quit immediately.")
+  "Non-nil means `bookmark-bmenu-search' quits immediately.")
 \f
 ;; Helper functions and macros.
 
@@ -318,8 +319,8 @@ with-buffer-modified-unmodified
 ;; Everyone else should go through them.
 
 (defun bookmark-name-from-full-record (bookmark-record)
-  "Return the name of BOOKMARK-RECORD.  BOOKMARK-RECORD is, e.g.,
-one element from `bookmark-alist'."
+  "Return the name of BOOKMARK-RECORD.
+BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'."
   (car bookmark-record))
 
 
@@ -346,8 +347,8 @@ bookmark-get-bookmark
 
 
 (defun bookmark-get-bookmark-record (bookmark-name-or-record)
-  "Return the record portion of the entry for BOOKMARK-NAME-OR-RECORD in
-`bookmark-alist' (that is, all information but the name)."
+  "Return the record portion of BOOKMARK-NAME-OR-RECORD in `bookmark-alist'.
+In other words, return all information but the name."
   (let ((alist (cdr (bookmark-get-bookmark bookmark-name-or-record))))
     ;; The bookmark objects can either look like (NAME ALIST) or
     ;; (NAME . ALIST), so we have to distinguish the two here.
@@ -812,7 +813,7 @@ bookmark-set-internal
            (cond
             ((eq overwrite-or-push nil)
              (if (bookmark-get-bookmark str t)
-                 (error "A bookmark named \"%s\" already exists." str)
+                 (error "A bookmark named \"%s\" already exists" str)
                (bookmark-store str (cdr record) nil)))
             ((eq overwrite-or-push 'overwrite)
              (bookmark-store str (cdr record) nil))
@@ -937,6 +938,7 @@ bookmark-edit-annotation-mode-map
   "Keymap for editing an annotation of a bookmark.")
 
 (defun bookmark-insert-annotation (bookmark-name-or-record)
+  "Insert annotation for BOOKMARK-NAME-OR-RECORD at point."
   (insert (funcall bookmark-edit-annotation-text-func bookmark-name-or-record))
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (if (and annotation (not (string-equal annotation "")))
@@ -1066,8 +1068,8 @@ bookmark-after-jump-hook
 Useful for example to unhide text in `outline-mode'.")
 
 (defun bookmark--jump-via (bookmark-name-or-record display-function)
-  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION with
-current buffer as argument.
+  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION.
+DISPLAY-FUNCTION is called with the current buffer as argument.
 
 After calling DISPLAY-FUNCTION, set window point to the point specified
 by BOOKMARK-NAME-OR-RECORD, if necessary, run `bookmark-after-jump-hook',
@@ -1676,7 +1678,7 @@ 'list-bookmarks
 (defalias 'edit-bookmarks 'bookmark-bmenu-list)
 
 (defun bookmark-bmenu-set-header ()
-  "Sets the immutable header line."
+  "Set the immutable header line."
   (let ((header (concat "%% " "Bookmark")))
     (when bookmark-bmenu-toggle-filenames
       (setq header (concat header
@@ -1836,8 +1838,8 @@ bookmark-bmenu-bookmark
 
 
 (defun bookmark-show-annotation (bookmark-name-or-record)
-  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer,
-if an annotation exists."
+  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer.
+If the annotation does not exist, do nothing."
   (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
     (when (and annotation (not (string-equal annotation "")))
       (save-excursion
@@ -1998,7 +2000,9 @@ bookmark-bmenu-switch-other-window
     (bookmark--jump-via bookmark fun)))
 
 (defun bookmark-bmenu-other-window-with-mouse (event)
-  "Select bookmark at the mouse pointer in other window, leaving bookmark menu visible."
+  "Jump to bookmark at mouse EVENT position in other window.
+Move point in menu buffer to the position of EVENT and leave
+bookmark menu visible."
   (interactive "e")
   (with-current-buffer (window-buffer (posn-window (event-end event)))
     (save-excursion
@@ -2124,8 +2128,8 @@ bookmark-bmenu-locate
     (message "%s" (bookmark-location bmrk))))
 
 (defun bookmark-bmenu-relocate ()
-  "Change the file path of the bookmark on the current line,
-  prompting with completion for the new path."
+  "Change the absolute file name of the bookmark on the current line.
+Prompt with completion for the new path."
   (interactive)
   (let ((bmrk (bookmark-bmenu-bookmark))
         (thispoint (point)))
-- 
2.21.0


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

* bug#35916: [PATCH] checkdoc fixes in bookmark.el
  2019-06-08 17:22         ` Stefan Kangas
@ 2019-06-09  6:51           ` Eli Zaretskii
  0 siblings, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2019-06-09  6:51 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 35916-done

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 8 Jun 2019 19:22:38 +0200
> Cc: 35916@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > The first line of the doc string should mention the arguments, in this
> > case EVENT.  Could you please rework this hunk and resubmit the patch?
> 
> Sure.  I've attached a patch with the following fix:

Thanks, pushed to the master branch.





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

end of thread, other threads:[~2019-06-09  6:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-26 10:59 bug#35916: [PATCH] checkdoc fixes in bookmark.el Stefan Kangas
2019-05-27  3:52 ` Drew Adams
2019-05-27 21:36   ` Stefan Kangas
2019-05-30 17:53     ` Stefan Kangas
2019-06-08  8:37       ` Eli Zaretskii
2019-06-08 17:22         ` Stefan Kangas
2019-06-09  6:51           ` Eli Zaretskii

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).