From 2e67ad18e926935107b899be61b7b16b03250906 Mon Sep 17 00:00:00 2001 From: Jeetaditya Chatterjee Date: Sat, 5 Aug 2023 23:14:43 +0530 Subject: [PATCH 1/5] feat(emacs-lisp): no doc warnings with flymake --- modules/lang/emacs-lisp/autoload.el | 68 +++++++++++++++++------------ modules/lang/emacs-lisp/config.el | 10 ++--- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/modules/lang/emacs-lisp/autoload.el b/modules/lang/emacs-lisp/autoload.el index b2e650320..616fd1676 100644 --- a/modules/lang/emacs-lisp/autoload.el +++ b/modules/lang/emacs-lisp/autoload.el @@ -290,39 +290,49 @@ Essentially, this means in any elisp file that either: This generally applies to your private config (`doom-user-dir') or Doom's source \(`doom-emacs-dir')." :since "3.0.0" - (unless (and (bound-and-true-p flycheck-mode) - (not (+emacs-lisp--in-package-buffer-p))) + (unless (and (or (bound-and-true-p flycheck-mode) + (bound-and-true-p flymake-mode)) + (not (+emacs-lisp--in-package-buffer-p))) (setq +emacs-lisp-non-package-mode nil)) (when (derived-mode-p 'emacs-lisp-mode) (add-hook 'after-save-hook #'+emacs-lisp-non-package-mode nil t)) (if (not +emacs-lisp-non-package-mode) - (when (get 'flycheck-disabled-checkers 'initial-value) - (setq-local flycheck-disabled-checkers (get 'flycheck-disabled-checkers 'initial-value)) - (kill-local-variable 'flycheck-emacs-lisp-check-form)) - (with-memoization (get 'flycheck-disabled-checkers 'initial-value) - flycheck-disabled-checkers) - (setq-local flycheck-emacs-lisp-check-form - (prin1-to-string - `(progn - (setq doom-modules ',doom-modules - doom-disabled-packages ',doom-disabled-packages - byte-compile-warnings ',+emacs-lisp-linter-warnings) - (condition-case e - (progn - (require 'doom) - (require 'doom-cli) - (require 'doom-start)) - (error - (princ - (format "%s:%d:%d:Error:Failed to load Doom: %s\n" - (or ,(ignore-errors - (file-name-nondirectory - (buffer-file-name (buffer-base-buffer)))) - (car command-line-args-left)) - 0 0 (error-message-string e))))) - ,(read (default-toplevel-value 'flycheck-emacs-lisp-check-form)))) - flycheck-disabled-checkers (cons 'emacs-lisp-checkdoc - flycheck-disabled-checkers)))) + (if (modulep! :checkers syntax +flymake) + ;; flymake + (progn + (add-hook 'flymake-diagnostic-functions #'elisp-flymake-checkdoc nil t)) + ;; flycheck + (when (get 'flycheck-disabled-checkers 'initial-value) + (setq-local flycheck-disabled-checkers (get 'flycheck-disabled-checkers 'initial-value)) + (kill-local-variable 'flycheck-emacs-lisp-check-form))) + (if (modulep! :checkers syntax +flymake) + ;; flymake + (remove-hook 'flymake-diagnostic-functions #'elisp-flymake-checkdoc t) + ;; flycheck + (with-memoization (get 'flycheck-disabled-checkers 'initial-value) + flycheck-disabled-checkers) + (setq-local flycheck-emacs-lisp-check-form + (prin1-to-string + `(progn + (setq doom-modules ',doom-modules + doom-disabled-packages ',doom-disabled-packages + byte-compile-warnings ',+emacs-lisp-linter-warnings) + (condition-case e + (progn + (require 'doom) + (require 'doom-cli) + (require 'doom-start)) + (error + (princ + (format "%s:%d:%d:Error:Failed to load Doom: %s\n" + (or ,(ignore-errors + (file-name-nondirectory + (buffer-file-name (buffer-base-buffer)))) + (car command-line-args-left)) + 0 0 (error-message-string e))))) + ,(read (default-toplevel-value 'flycheck-emacs-lisp-check-form)))) + flycheck-disabled-checkers (cons 'emacs-lisp-checkdoc + flycheck-disabled-checkers))))) ;; diff --git a/modules/lang/emacs-lisp/config.el b/modules/lang/emacs-lisp/config.el index 1e140a00e..4cb0e53c6 100644 --- a/modules/lang/emacs-lisp/config.el +++ b/modules/lang/emacs-lisp/config.el @@ -95,11 +95,11 @@ See `+emacs-lisp-non-package-mode' for details.") ;; Ensure straight sees modifications to installed packages #'+emacs-lisp-init-straight-maybe-h) - ;; UX: Flycheck's two emacs-lisp checkers produce a *lot* of false positives - ;; in non-packages (like Emacs configs or elisp scripts), so I disable - ;; `emacs-lisp-checkdoc' and set `byte-compile-warnings' to a subset of the - ;; original in the flycheck instance (see `+emacs-lisp-linter-warnings'). - (add-hook 'flycheck-mode-hook #'+emacs-lisp-non-package-mode) + ;; UX: Both Flycheck's and Flymake's two + ;; emacs-lisp checkers produce a *lot* of false positives in non-packages + ;; (like Emacs configs or elisp scripts), so I disable `checkdoc' (`emacs-lisp-checkdoc', `elisp-flymake-checkdoc') + ;; and set `byte-compile-warnings' to a subset that makes more sense (see `+emacs-lisp-linter-warnings') + (add-hook! '(flycheck-mode-hook flymake-mode-hook) #'+emacs-lisp-non-package-mode) ;; Enhance elisp syntax highlighting, by highlighting Doom-specific ;; constructs, defined symbols, and truncating :pin's in `package!' calls. From e9763514241583be864cbc2caaa6f6dee8c9907e Mon Sep 17 00:00:00 2001 From: Jeetaditya Chatterjee Date: Mon, 7 Aug 2023 23:38:46 +0530 Subject: [PATCH 2/5] feat(emacs-lisp): add reduced-flymake-byte-compile This is a flymake backend that gives a reduced set of byte compiler warnings in accordance too `+emacs-lisp-linter-warnings` --- modules/lang/emacs-lisp/autoload.el | 83 +++++++++++++++++++++++++---- 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/modules/lang/emacs-lisp/autoload.el b/modules/lang/emacs-lisp/autoload.el index 616fd1676..d7e838ff7 100644 --- a/modules/lang/emacs-lisp/autoload.el +++ b/modules/lang/emacs-lisp/autoload.el @@ -277,9 +277,71 @@ https://emacs.stackexchange.com/questions/10230/how-to-indent-keywords-aligned" file-base))))))) (not (locate-dominating-file default-directory ".doommodule"))))) +(defvar-local +emacs-lisp-reduced-flymake-byte-compile--process nil) + +(defun +emacs-lisp-reduced-flymake-byte-compile (report-fn &rest _args) + "A Flymake backend for warnings from the elisp byte compiler + +This checker reduces the amount of false positives the byte compiler throws off +compared too `elisp-flymake-byte-compile'. The linter warnings that are enabled +are set by `+emacs-lisp-linter-warnings' + +This backend does not need to be added directly +as `+emacs-lisp-non-package-mode' will enable it and disable the other checkers." + ;; if a process already exists. kill it. + (when (and +emacs-lisp-reduced-flymake-byte-compile--process + (process-live-p +emacs-lisp-reduced-flymake-byte-compile--process)) + (kill-process +emacs-lisp-reduced-flymake-byte-compile--process)) + (let ((source (current-buffer)) + (tmp-file (make-temp-file "+emacs-lisp-byte-compile-src")) + (out-buf (generate-new-buffer "+emacs-lisp-byte-compile-out"))) + ;; write the content to a temp file + (save-restriction + (widen) + (write-region nil nil tmp-file nil 'nomessage)) + ;; make the process + (setq +emacs-lisp-reduced-flymake-byte-compile--process + (make-process + :name "+emacs-reduced-flymake" + :noquery t + :connetion-type 'pipe + :buffer out-buf + :command `(,(expand-file-name invocation-name invocation-directory) + "-Q" + "--batch" + ,@(mapcan (fn! (list "-L" %)) elisp-flymake-byte-compile-load-path) + ;; this is what silences the byte compiler + "--eval" ,(prin1-to-string `(setq doom-modules ',doom-modules + doom-disabled-packages ',doom-disabled-packages + byte-compile-warnings ',+emacs-lisp-linter-warnings) +) + "-f" "elisp-flymake--batch-compile-for-flymake" + ,tmp-file) + :stderr "*stderr of +elisp-flymake-byte-compile-out*" + :sentinel + ;; deal with the process when it exits + (lambda (proc _event) + (when (memq (process-status proc) '(exit signal)) + (unwind-protect + (cond + ;; if the buffer is dead or the process is not the same, log the process as old. + ((or (not (buffer-live-p source)) + (not (with-current-buffer source (eq proc +emacs-lisp-reduced-flymake-byte-compile--process)))) + (flymake-log :warning "byte compile process %s is old" proc)) + ;; if the process exited without problem process the buffer + ((zerop (process-exit-status proc)) + (elisp-flymake--byte-compile-done report-fn source out-buf)) + ;; otherwise something else horrid has gone wrong and we panic + (t (funcall report-fn :panic + :explanation + (format "byte compile process %s died" proc)))) + ;; cleanup + (ignore-errors (delete-file tmp-file)) + (kill-buffer out-buf)))))))) + ;;;###autoload (define-minor-mode +emacs-lisp-non-package-mode - "Reduce flycheck verbosity where it is appropriate. + "Reduce flycheck/flymake verbosity where it is appropriate. Essentially, this means in any elisp file that either: - Is not a theme in `custom-theme-load-path', @@ -292,7 +354,7 @@ This generally applies to your private config (`doom-user-dir') or Doom's source :since "3.0.0" (unless (and (or (bound-and-true-p flycheck-mode) (bound-and-true-p flymake-mode)) - (not (+emacs-lisp--in-package-buffer-p))) + (not (+emacs-lisp--in-package-buffer-p))) (setq +emacs-lisp-non-package-mode nil)) (when (derived-mode-p 'emacs-lisp-mode) (add-hook 'after-save-hook #'+emacs-lisp-non-package-mode nil t)) @@ -300,14 +362,17 @@ This generally applies to your private config (`doom-user-dir') or Doom's source (if (modulep! :checkers syntax +flymake) ;; flymake (progn - (add-hook 'flymake-diagnostic-functions #'elisp-flymake-checkdoc nil t)) - ;; flycheck - (when (get 'flycheck-disabled-checkers 'initial-value) - (setq-local flycheck-disabled-checkers (get 'flycheck-disabled-checkers 'initial-value)) - (kill-local-variable 'flycheck-emacs-lisp-check-form))) + (add-hook! 'flymake-diagnostic-functions :local #'elisp-flymake-checkdoc #'elisp-flymake-byte-compile) + (remove-hook 'flymake-diagnostic-functions #'+emacs-lisp-reduced-flymake-byte-compile)) + ;; flycheck + (when (get 'flycheck-disabled-checkers 'initial-value) + (setq-local flycheck-disabled-checkers (get 'flycheck-disabled-checkers 'initial-value)) + (kill-local-variable 'flycheck-emacs-lisp-check-form))) (if (modulep! :checkers syntax +flymake) - ;; flymake - (remove-hook 'flymake-diagnostic-functions #'elisp-flymake-checkdoc t) + ;; flymake + (progn + (remove-hook! 'flymake-diagnostic-functions :local #'elisp-flymake-checkdoc #'elisp-flymake-byte-compile) + (add-hook 'flymake-diagnostic-functions #'+emacs-lisp-reduced-flymake-byte-compile)) ;; flycheck (with-memoization (get 'flycheck-disabled-checkers 'initial-value) flycheck-disabled-checkers) From c37f314f14b71034328193397af2b44136fae276 Mon Sep 17 00:00:00 2001 From: Jeetaditya Chatterjee Date: Sun, 10 Sep 2023 23:32:26 +0100 Subject: [PATCH 3/5] fix(syntax): s/connetion/connection Also rework docstring and s/too/to --- modules/lang/emacs-lisp/autoload.el | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/modules/lang/emacs-lisp/autoload.el b/modules/lang/emacs-lisp/autoload.el index d7e838ff7..91025667d 100644 --- a/modules/lang/emacs-lisp/autoload.el +++ b/modules/lang/emacs-lisp/autoload.el @@ -280,10 +280,10 @@ https://emacs.stackexchange.com/questions/10230/how-to-indent-keywords-aligned" (defvar-local +emacs-lisp-reduced-flymake-byte-compile--process nil) (defun +emacs-lisp-reduced-flymake-byte-compile (report-fn &rest _args) - "A Flymake backend for warnings from the elisp byte compiler + "A Flymake backend for byte compilation in non-package elisp files. This checker reduces the amount of false positives the byte compiler throws off -compared too `elisp-flymake-byte-compile'. The linter warnings that are enabled +compared to `elisp-flymake-byte-compile'. The linter warnings that are enabled are set by `+emacs-lisp-linter-warnings' This backend does not need to be added directly @@ -304,7 +304,7 @@ as `+emacs-lisp-non-package-mode' will enable it and disable the other checkers. (make-process :name "+emacs-reduced-flymake" :noquery t - :connetion-type 'pipe + :connection-type 'pipe :buffer out-buf :command `(,(expand-file-name invocation-name invocation-directory) "-Q" @@ -313,8 +313,7 @@ as `+emacs-lisp-non-package-mode' will enable it and disable the other checkers. ;; this is what silences the byte compiler "--eval" ,(prin1-to-string `(setq doom-modules ',doom-modules doom-disabled-packages ',doom-disabled-packages - byte-compile-warnings ',+emacs-lisp-linter-warnings) -) + byte-compile-warnings ',+emacs-lisp-linter-warnings)) "-f" "elisp-flymake--batch-compile-for-flymake" ,tmp-file) :stderr "*stderr of +elisp-flymake-byte-compile-out*" From b50b2c464004452d348e364865863b2c4fe542d8 Mon Sep 17 00:00:00 2001 From: Jeetaditya Chatterjee Date: Sun, 10 Sep 2023 23:34:05 +0100 Subject: [PATCH 4/5] refactor(syntax): separate out implementations As `+emacs-lisp-non-package-mode` handles both flymake and flycheck, making two internal modes that then `+emacs-lisp-non-package-mode` calls, makes the code cleaner --- modules/lang/emacs-lisp/autoload.el | 83 +++++++++++++++-------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/modules/lang/emacs-lisp/autoload.el b/modules/lang/emacs-lisp/autoload.el index 91025667d..1dc48759b 100644 --- a/modules/lang/emacs-lisp/autoload.el +++ b/modules/lang/emacs-lisp/autoload.el @@ -338,6 +338,47 @@ as `+emacs-lisp-non-package-mode' will enable it and disable the other checkers. (ignore-errors (delete-file tmp-file)) (kill-buffer out-buf)))))))) +(define-minor-mode +emacs-lisp--flymake-non-package-mode + "" + :since "3.0.0" + (if +emacs-lisp--flymake-non-package-mode + (progn + (remove-hook! 'flymake-diagnostic-functions :local #'elisp-flymake-checkdoc #'elisp-flymake-byte-compile) + (add-hook 'flymake-diagnostic-functions #'+emacs-lisp-reduced-flymake-byte-compile nil t)) + (add-hook! 'flymake-diagnostic-functions :local #'elisp-flymake-checkdoc #'elisp-flymake-byte-compile) + (remove-hook 'flymake-diagnostic-functions #'+emacs-lisp-reduced-flymake-byte-compile t))) + +(define-minor-mode +emacs-lisp--flycheck-non-package-mode + "" + :since "3.0.0" + (if (not +emacs-lisp--flycheck-non-package-mode) + (when (get 'flycheck-disabled-checkers 'initial-value) + (setq-local flycheck-disabled-checkers (get 'flycheck-disabled-checkers 'initial-value)) + (kill-local-variable 'flycheck-emacs-lisp-check-form)) + (with-memoization (get 'flycheck-disabled-checkers 'initial-value) + flycheck-disabled-checkers) + (setq-local flycheck-emacs-lisp-check-form + (prin1-to-string + `(progn + (setq doom-modules ',doom-modules + doom-disabled-packages ',doom-disabled-packages + byte-compile-warnings ',+emacs-lisp-linter-warnings) + (condition-case e + (progn + (require 'doom) + (require 'doom-cli) + (require 'doom-start)) + (error + (princ + (format "%s:%d:%d:Error:Failed to load Doom: %s\n" + (or ,(ignore-errors + (file-name-nondirectory + (buffer-file-name (buffer-base-buffer)))) + (car command-line-args-left)) + 0 0 (error-message-string e))))) + ,(read (default-toplevel-value 'flycheck-emacs-lisp-check-form)))) + flycheck-disabled-checkers (cons 'emacs-lisp-checkdoc + flycheck-disabled-checkers)))) ;;;###autoload (define-minor-mode +emacs-lisp-non-package-mode "Reduce flycheck/flymake verbosity where it is appropriate. @@ -357,46 +398,10 @@ This generally applies to your private config (`doom-user-dir') or Doom's source (setq +emacs-lisp-non-package-mode nil)) (when (derived-mode-p 'emacs-lisp-mode) (add-hook 'after-save-hook #'+emacs-lisp-non-package-mode nil t)) - (if (not +emacs-lisp-non-package-mode) - (if (modulep! :checkers syntax +flymake) - ;; flymake - (progn - (add-hook! 'flymake-diagnostic-functions :local #'elisp-flymake-checkdoc #'elisp-flymake-byte-compile) - (remove-hook 'flymake-diagnostic-functions #'+emacs-lisp-reduced-flymake-byte-compile)) - ;; flycheck - (when (get 'flycheck-disabled-checkers 'initial-value) - (setq-local flycheck-disabled-checkers (get 'flycheck-disabled-checkers 'initial-value)) - (kill-local-variable 'flycheck-emacs-lisp-check-form))) + (let ((toggle (if +emacs-lisp-non-package-mode +1 -1))) (if (modulep! :checkers syntax +flymake) - ;; flymake - (progn - (remove-hook! 'flymake-diagnostic-functions :local #'elisp-flymake-checkdoc #'elisp-flymake-byte-compile) - (add-hook 'flymake-diagnostic-functions #'+emacs-lisp-reduced-flymake-byte-compile)) - ;; flycheck - (with-memoization (get 'flycheck-disabled-checkers 'initial-value) - flycheck-disabled-checkers) - (setq-local flycheck-emacs-lisp-check-form - (prin1-to-string - `(progn - (setq doom-modules ',doom-modules - doom-disabled-packages ',doom-disabled-packages - byte-compile-warnings ',+emacs-lisp-linter-warnings) - (condition-case e - (progn - (require 'doom) - (require 'doom-cli) - (require 'doom-start)) - (error - (princ - (format "%s:%d:%d:Error:Failed to load Doom: %s\n" - (or ,(ignore-errors - (file-name-nondirectory - (buffer-file-name (buffer-base-buffer)))) - (car command-line-args-left)) - 0 0 (error-message-string e))))) - ,(read (default-toplevel-value 'flycheck-emacs-lisp-check-form)))) - flycheck-disabled-checkers (cons 'emacs-lisp-checkdoc - flycheck-disabled-checkers))))) + (+emacs-lisp--flymake-non-package-mode toggle) + (+emacs-lisp--flycheck-non-package-mode toggle)))) ;; From a7b810a93f05e4966d3dcce207c7c2ce77bd4039 Mon Sep 17 00:00:00 2001 From: Jeetaditya Chatterjee Date: Sun, 15 Oct 2023 19:33:07 +0100 Subject: [PATCH 5/5] fix(emacs-lisp): lexical argument, error popup this was caused by the use of fn! and an argument which interacted badly with doom snippets of all things. --- modules/lang/emacs-lisp/autoload.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/lang/emacs-lisp/autoload.el b/modules/lang/emacs-lisp/autoload.el index 1dc48759b..a94b8e4b2 100644 --- a/modules/lang/emacs-lisp/autoload.el +++ b/modules/lang/emacs-lisp/autoload.el @@ -309,7 +309,7 @@ as `+emacs-lisp-non-package-mode' will enable it and disable the other checkers. :command `(,(expand-file-name invocation-name invocation-directory) "-Q" "--batch" - ,@(mapcan (fn! (list "-L" %)) elisp-flymake-byte-compile-load-path) + ,@(mapcan (lambda (p) (list "-L" p)) elisp-flymake-byte-compile-load-path) ;; this is what silences the byte compiler "--eval" ,(prin1-to-string `(setq doom-modules ',doom-modules doom-disabled-packages ',doom-disabled-packages