GNU bug report logs - #30819
[Bug #19655] Patches for GNU Emacs

Previous Next

Full log

View this message in rfc822 format

From: martin rudalics <rudalics <at> gmx.at>
To: Simona Arizanova <simonikane <at> hotmail.fr>, 30819 <at> debbugs.gnu.org
Subject: bug#30819: [Bug  #19655] Patches for GNU Emacs
Date: Mon, 26 Mar 2018 11:01:42 +0200

Please always make sure to send a copy to the bug address too.

> I'm sorry for the delay.

My apologies.  I was delayed as well.

> Here I joined a text file that contains the diffs in the correct order.

Thank you.  But we now lost the identification of the file to be
patched.  Please try to make sure that the beginning of such a patch
appears like

--- a/lisp/help.el	2018-03-03 15:09:47.652000000 +0100
+++ b/lisp/help.el	2018-03-09 20:14:31.492000000 +0100


A few minor comments below:


-(defun search-forward-help-for-help()

Please always leave one space between function name and arguments as
in:

-(defun search-forward-help-for-help ()


-  "Allows to research forward a string into the *Metahelp* window"

Please always use active voice for doc-strings and terminate them with
a period as:

-  "Search forward *Metahelp* window."

(I think you really mean search and not "re"search forward here.)
Maybe we also should simply say "help window" instead of " *Metahelp*
window".  There is no well established nomenclature for this object
and few people have an idea what *Metahelp* stands for (and whether it
contains a leading space).


-  (interactive)
-  ;;Moves the cursor to the *Metahelp* window

Please always leave a space after a comment delimiter and use active
voice for comments as well and terminate them with a period as:

-  ;; Move cursor to the *Metahelp* window.

The

-  ;; Do incremental search forward.

below is well written in this regard.



-  (switch-to-buffer-other-window " *Metahelp*")

Can you add a comment that explains why you use
'switch-to-buffer-other-window' (and not, for example,
'pop-to-buffer') here?


-  (isearch-forward nil 1))

Please always use nil or t for inherently boolean arguments like
NO-RECURSIVE-EDIT, so

-  (isearch-forward nil t))

is preferable here.


Also, I assume that you have not signed legal papers for Emacs so we
have to mark this as a tiny change.  Correct?

Thanks again for working on this, martin

Previous Next

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