GNU bug report logs - #35916
[PATCH] checkdoc fixes in bookmark.el

Previous Next

Package: emacs;

Reported by: Stefan Kangas <stefan <at> marxist.se>

Date: Sun, 26 May 2019 15:36:02 UTC

Severity: minor

Tags: patch

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35916 in the body.
You can then email your comments to 35916 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#35916; Package emacs. (Sun, 26 May 2019 15:36:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Stefan Kangas <stefan <at> marxist.se>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 26 May 2019 15:36:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] checkdoc fixes in bookmark.el
Date: Sun, 26 May 2019 12:59:11 +0200
[Message part 1 (text/plain, inline)]
I ran checkdoc on bookmark.el and fixed some of the errors.
Please see attached patch.

Thanks,
Stefan Kangas
[Message part 2 (text/html, inline)]
[checkdoc-fixes-in-bookmark.el.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35916; Package emacs. (Mon, 27 May 2019 03:53:02 GMT) Full text and rfc822 format available.

Message #8 received at 35916 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Kangas <stefan <at> marxist.se>, 35916 <at> debbugs.gnu.org
Subject: RE: bug#35916: [PATCH] checkdoc fixes in bookmark.el
Date: Sun, 26 May 2019 20:52:42 -0700 (PDT)
(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.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35916; Package emacs. (Mon, 27 May 2019 21:37:01 GMT) Full text and rfc822 format available.

Message #11 received at 35916 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 35916 <at> debbugs.gnu.org
Subject: Re: bug#35916: [PATCH] checkdoc fixes in bookmark.el
Date: Mon, 27 May 2019 23:36:00 +0200
[Message part 1 (text/plain, inline)]
Drew Adams <drew.adams <at> 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
[0001-Checkdoc-fixes-in-bookmark.el.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35916; Package emacs. (Thu, 30 May 2019 17:55:02 GMT) Full text and rfc822 format available.

Message #14 received at 35916 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
Cc: 35916 <at> debbugs.gnu.org
Subject: Re: bug#35916: [PATCH] checkdoc fixes in bookmark.el
Date: Thu, 30 May 2019 19:53:58 +0200
[Message part 1 (text/plain, inline)]
Please find attached an updated version of the patch with an improved commit
message.

Thanks,
Stefan Kangas
[0001-Checkdoc-fixes-in-lisp-bookmark.el.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35916; Package emacs. (Sat, 08 Jun 2019 08:38:02 GMT) Full text and rfc822 format available.

Message #17 received at 35916 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 35916 <at> debbugs.gnu.org
Subject: Re: bug#35916: [PATCH] checkdoc fixes in bookmark.el
Date: Sat, 08 Jun 2019 11:37:42 +0300
> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Thu, 30 May 2019 19:53:58 +0200
> Cc: 35916 <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35916; Package emacs. (Sat, 08 Jun 2019 17:23:01 GMT) Full text and rfc822 format available.

Message #20 received at 35916 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 35916 <at> debbugs.gnu.org
Subject: Re: bug#35916: [PATCH] checkdoc fixes in bookmark.el
Date: Sat, 8 Jun 2019 19:22:38 +0200
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> 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
[0001-Checkdoc-fixes-in-lisp-bookmark.el-4.patch (application/octet-stream, attachment)]

Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sun, 09 Jun 2019 06:52:02 GMT) Full text and rfc822 format available.

Notification sent to Stefan Kangas <stefan <at> marxist.se>:
bug acknowledged by developer. (Sun, 09 Jun 2019 06:52:02 GMT) Full text and rfc822 format available.

Message #25 received at 35916-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 35916-done <at> debbugs.gnu.org
Subject: Re: bug#35916: [PATCH] checkdoc fixes in bookmark.el
Date: Sun, 09 Jun 2019 09:51:36 +0300
> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Sat, 8 Jun 2019 19:22:38 +0200
> Cc: 35916 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> 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.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 07 Jul 2019 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 35 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.