bug#34290: [PATCH] checkdoc byte compile warnings

bug#34290: [PATCH] checkdoc byte compile warnings

[Alex Branham]

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

Hi all -

The attached patch fixes the byte compiler warnings that checkdoc issues
currently by declaring the functions for the byte compiler. It also
removes some old XEmacs compatibility code.

Thanks,
Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-byte-compile-warnings-in-checkdoc.el.patch --]
[-- Type: text/x-patch, Size: 2244 bytes --]

From 78ec520b6214d768d340d0ab07159605e1f1b3e1 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Sat, 2 Feb 2019 09:45:11 -0600
Subject: [PATCH] Fix byte compile warnings in checkdoc.el

* lisp/emacs-lisp/checkdoc.el (checkdoc-file-comments-engine): Fix
  byte compile warnings by declaring functions, remove XEmacs
  compatibility code.
---
 lisp/emacs-lisp/checkdoc.el | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index c0da61a589..1289e1cbc9 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -2205,21 +2205,17 @@ News agents may remove it"
 ;;
 (defvar generate-autoload-cookie)
 
-(eval-when-compile (require 'lisp-mnt))	; expand silly defsubsts
-(declare-function lm-summary "lisp-mnt" (&optional file))
-(declare-function lm-section-start "lisp-mnt" (header &optional after))
-(declare-function lm-section-end "lisp-mnt" (header))
+(declare-function lm-summary "lisp-mnt"  (&optional file))
+(declare-function lm-commentary-mark "lisp-mnt")
+(declare-function lm-history-mark "lisp-mnt")
+(declare-function lm-code-mark "lisp-mnt")
+(declare-function lm-commentary-end "lisp-mnt")
 
 (defun checkdoc-file-comments-engine ()
   "Return a message list if this file does not match the Emacs standard.
 This checks for style only, such as the first line, Commentary:,
 Code:, and others referenced in the style guide."
-  (if (featurep 'lisp-mnt)
-      nil
-    (require 'lisp-mnt)
-    ;; Old XEmacs don't have `lm-commentary-mark'
-    (if (and (not (fboundp 'lm-commentary-mark)) (fboundp 'lm-commentary))
-	(defalias 'lm-commentary-mark #'lm-commentary)))
+  (require 'lisp-mnt)
   (save-excursion
     (let* ((f1 (file-name-nondirectory (buffer-file-name)))
 	   (fn (file-name-sans-extension f1))
@@ -2280,7 +2276,7 @@ Code:, and others referenced in the style guide."
 	(if (or (not checkdoc-force-history-flag)
 		(file-exists-p "ChangeLog")
 		(file-exists-p "../ChangeLog")
-                (and (fboundp 'lm-history-mark) (funcall #'lm-history-mark)))
+                (lm-history-mark))
 	    nil
 	  (progn
 	    (goto-char (or (lm-commentary-mark) (point-min)))
-- 
2.19.2

bug#34290: [PATCH] checkdoc byte compile warnings

[Eli Zaretskii]

> From: Alex Branham <alex.branham@gmail.com>
> Date: Sat, 02 Feb 2019 09:47:45 -0600
> 
> The attached patch fixes the byte compiler warnings that checkdoc issues
> currently by declaring the functions for the byte compiler. It also
> removes some old XEmacs compatibility code.

Thanks.  Can you explain the purpose of requiring lisp-mnt inside a
function, as opposed to at the top level?  The latter would then
remove the need for using declare-function, I think.

AFAIU, declare-function is useful when the function in question is
auto-loaded, but we don't want to load its package unconditionally,
e.g. because that function is used only in a small part of the package
being compiled.  None of which seems to be the case here, or did I
miss something?

bug#34290: [PATCH] checkdoc byte compile warnings

[Alex Branham]

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


On Sat 02 Feb 2019 at 10:06, Eli Zaretskii <eliz@gnu.org> wrote:

> AFAIU, declare-function is useful when the function in question is
> auto-loaded, but we don't want to load its package unconditionally,
> e.g. because that function is used only in a small part of the package
> being compiled.  None of which seems to be the case here, or did I
> miss something?

I was just following with what the file was already doing. More than
happy to require lisp-mnt top-level though, patch attached.

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Fix-byte-compile-warnings-in-checkdoc.el.patch --]
[-- Type: text/x-patch, Size: 2295 bytes --]

From 3327f9d74f6e063104726ec7aa4df805aa51decd Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Sat, 2 Feb 2019 09:45:11 -0600
Subject: [PATCH] Fix byte compile warnings in checkdoc.el

* lisp/emacs-lisp/checkdoc.el (checkdoc-file-comments-engine): Fix
  byte compile warnings by requiring lisp-mnt top-level, remove XEmacs
  compatibility code.
---
 lisp/emacs-lisp/checkdoc.el | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index c0da61a589..dca2f16956 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -174,6 +174,7 @@
 (require 'cl-lib)
 (require 'help-mode) ;; for help-xref-info-regexp
 (require 'thingatpt) ;; for handy thing-at-point-looking-at
+(require 'lisp-mnt)
 
 (defvar compilation-error-regexp-alist)
 (defvar compilation-mode-font-lock-keywords)
@@ -2205,21 +2206,10 @@ News agents may remove it"
 ;;
 (defvar generate-autoload-cookie)
 
-(eval-when-compile (require 'lisp-mnt))	; expand silly defsubsts
-(declare-function lm-summary "lisp-mnt" (&optional file))
-(declare-function lm-section-start "lisp-mnt" (header &optional after))
-(declare-function lm-section-end "lisp-mnt" (header))
-
 (defun checkdoc-file-comments-engine ()
   "Return a message list if this file does not match the Emacs standard.
 This checks for style only, such as the first line, Commentary:,
 Code:, and others referenced in the style guide."
-  (if (featurep 'lisp-mnt)
-      nil
-    (require 'lisp-mnt)
-    ;; Old XEmacs don't have `lm-commentary-mark'
-    (if (and (not (fboundp 'lm-commentary-mark)) (fboundp 'lm-commentary))
-	(defalias 'lm-commentary-mark #'lm-commentary)))
   (save-excursion
     (let* ((f1 (file-name-nondirectory (buffer-file-name)))
 	   (fn (file-name-sans-extension f1))
@@ -2280,7 +2270,7 @@ Code:, and others referenced in the style guide."
 	(if (or (not checkdoc-force-history-flag)
 		(file-exists-p "ChangeLog")
 		(file-exists-p "../ChangeLog")
-                (and (fboundp 'lm-history-mark) (funcall #'lm-history-mark)))
+                (lm-history-mark))
 	    nil
 	  (progn
 	    (goto-char (or (lm-commentary-mark) (point-min)))
-- 
2.19.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#34290: [PATCH] checkdoc byte compile warnings

[Eli Zaretskii]

> From: Alex Branham <alex.branham@gmail.com>
> Cc: 34290@debbugs.gnu.org
> Date: Sat, 02 Feb 2019 10:28:13 -0600
> 
> On Sat 02 Feb 2019 at 10:06, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > AFAIU, declare-function is useful when the function in question is
> > auto-loaded, but we don't want to load its package unconditionally,
> > e.g. because that function is used only in a small part of the package
> > being compiled.  None of which seems to be the case here, or did I
> > miss something?
> 
> I was just following with what the file was already doing. More than
> happy to require lisp-mnt top-level though, patch attached.

Thanks, this looks good.  Let's wait for a few days for additional
comments, if someone would like to voice them.

bug#34290: [PATCH] checkdoc byte compile warnings

[Alex Branham]

On Sat 02 Feb 2019 at 10:35, Eli Zaretskii <eliz@gnu.org> wrote:

> Thanks, this looks good.  Let's wait for a few days for additional
> comments, if someone would like to voice them.

Pushed as 1e155dcc8dcbaed926a1574bc543d404d2859866