bug#34290: [PATCH] checkdoc byte compile warnings

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#34290: [PATCH] checkdoc byte compile warnings

Alex Branham
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


0001-Fix-byte-compile-warnings-in-checkdoc.el.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#34290: [PATCH] checkdoc byte compile warnings

Eli Zaretskii
> From: Alex Branham <[hidden email]>
> 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?



Reply | Threaded
Open this post in threaded view
|

bug#34290: [PATCH] checkdoc byte compile warnings

Alex Branham

On Sat 02 Feb 2019 at 10:06, Eli Zaretskii <[hidden email]> 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


From 3327f9d74f6e063104726ec7aa4df805aa51decd Mon Sep 17 00:00:00 2001
From: Alex Branham <[hidden email]>
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


signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#34290: [PATCH] checkdoc byte compile warnings

Eli Zaretskii
> From: Alex Branham <[hidden email]>
> Cc: [hidden email]
> Date: Sat, 02 Feb 2019 10:28:13 -0600
>
> On Sat 02 Feb 2019 at 10:06, Eli Zaretskii <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|

bug#34290: [PATCH] checkdoc byte compile warnings

Alex Branham

On Sat 02 Feb 2019 at 10:35, Eli Zaretskii <[hidden email]> 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