From 575c3cccd9bd79ea529870ab5a6c709e320c0247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Sun, 9 Sep 2018 21:01:53 +0100 Subject: [PATCH 1/7] make compile: exit with non-zero code on error This would allow the CI to fail to compilation errors. --- bin/doom | 3 ++- core/cli/byte-compile.el | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bin/doom b/bin/doom index 3e25a819d..3a1fc43f5 100755 --- a/bin/doom +++ b/bin/doom @@ -103,4 +103,5 @@ "report, please include it!\n\n" "Emacs outputs to standard error, so you'll need to redirect stderr to\n" "stdout to pipe this to a file or clipboard!\n\n" - " e.g. doom -d install 2>&1 | clipboard-program"))))))))) + " e.g. doom -d install 2>&1 | clipboard-program")) + (signal 'doom-error e)))))))) diff --git a/core/cli/byte-compile.el b/core/cli/byte-compile.el index c14e3a7c6..fcd46e9d3 100644 --- a/core/cli/byte-compile.el +++ b/core/cli/byte-compile.el @@ -151,7 +151,9 @@ If RECOMPILE-P is non-nil, only recompile out-of-date files." "%s %d/%d file(s) (%d ignored)")) (if recompile-p "Recompiled" "Compiled") total-ok (- (length target-files) total-noop) - total-noop)) + total-noop) + (or (= total-fail 0) + (error "Failed to compile some files"))) ((debug error) (print! (red "\nThere were breaking errors.\n\n%s") "Reverting changes...") From 6eb95c98eafc3ac8ace291b685289a87083fd8e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Sun, 9 Sep 2018 21:05:45 +0100 Subject: [PATCH 2/7] Fix `bin/doom -d compile`: org-attach-directory not defined --- modules/lang/org/+attach.el | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/lang/org/+attach.el b/modules/lang/org/+attach.el index 3a31dbdf1..eee6c0223 100644 --- a/modules/lang/org/+attach.el +++ b/modules/lang/org/+attach.el @@ -36,9 +36,6 @@ (advice-add #'org-download-enable :override #'ignore) :config - (setq-default org-download-image-dir org-attach-directory - org-download-heading-lvl nil - org-download-timestamp "_%Y%m%d_%H%M%S") (setq org-download-screenshot-method (cond (IS-MAC "screencapture -i %s") @@ -72,6 +69,9 @@ (defun +org|init-attach () (setq org-attach-directory (expand-file-name +org-attach-dir org-directory)) + (setq-default org-download-image-dir org-attach-directory + org-download-heading-lvl nil + org-download-timestamp "_%Y%m%d_%H%M%S") ;; A shorter link to attachments (add-to-list 'org-link-abbrev-alist (cons "attach" (abbreviate-file-name org-attach-directory))) From 9d445c8a1f79b7748f3b7de0bcd24d9018f894da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Sun, 9 Sep 2018 21:03:54 +0100 Subject: [PATCH 3/7] make compile: fix error in emacs-lisp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` In toplevel form: ../modules/lang/emacs-lisp/autoload.el:71:21:Error: Wrong type argument: listp, "~/.emacs.d/modules/lang/emacs-lisp/autoload" ✕ Failed to compile modules/lang/emacs-lisp/autoload.el ``` This apparently attempts to get compiled multiple times, avoid that by attempting to compile only if it is not compiled already. --- 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 98309b8fd..e8957ca5d 100644 --- a/modules/lang/emacs-lisp/autoload.el +++ b/modules/lang/emacs-lisp/autoload.el @@ -66,7 +66,7 @@ library/userland functions" ;; `+emacs-lisp-highlight-vars-and-faces' is a potentially expensive function ;; and should be byte-compiled, no matter what, to ensure it runs as fast as ;; possible: -(eval-when-compile +(when (not (byte-code-function-p (symbol-function '+emacs-lisp-highlight-vars-and-faces))) (with-no-warnings (byte-compile #'+emacs-lisp-highlight-vars-and-faces))) From b9c966ec54f0ba7b6ac53e078290ddb364a37cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Tue, 11 Sep 2018 21:24:56 +0100 Subject: [PATCH 4/7] Backport to Emacs 25: avoid (setf (alist-get ... 'equal)) We would need to use `'equal` for comparison, but Emacs 25 only allows `'eq`. Using `advice-add` to override `alist-get` does not work, because `setf` has special handling for `alist-get`. `repl.el`: Switch to a hash table which already supports multiple comparison functions, and changing of elements even in Emacs 25. `eshell/autoload/settings.el`: use conditional set-or-push. Drop `doom*alist-get`, it is unused now. Thanks to @hlissner for the reimplementation. --- core/core-lib.el | 19 +------------------ modules/emacs/eshell/autoload/settings.el | 7 +++++-- modules/feature/eval/autoload/repl.el | 18 +++++++++--------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/core/core-lib.el b/core/core-lib.el index af6408b36..0e764aede 100644 --- a/core/core-lib.el +++ b/core/core-lib.el @@ -10,24 +10,7 @@ ;; if-let and when-let are deprecated in Emacs 26+ in favor of their ;; if-let* variants, so we alias them for 25 users. (defalias 'if-let* #'if-let) - (defalias 'when-let* #'when-let) - - ;; `alist-get' doesn't have its 5th argument before Emacs 26 - (defun doom*alist-get (key alist &optional default remove testfn) - "Return the value associated with KEY in ALIST. -If KEY is not found in ALIST, return DEFAULT. -Use TESTFN to lookup in the alist if non-nil. Otherwise, use `assq'. - -This is a generalized variable suitable for use with `setf'. -When using it to set a value, optional argument REMOVE non-nil -means to remove KEY from ALIST if the new value is `eql' to DEFAULT." - (ignore remove) ;;Silence byte-compiler. - (let ((x (cond ((null testfn) (assq key alist)) - ((eq testfn #'equal) (assoc key alist)) - ((cl-find key alist :key #'car :test testfn))))) - (if x (cdr x) default))) - (advice-add #'alist-get :override #'doom*alist-get)))) - + (defalias 'when-let* #'when-let)))) ;; ;; Helpers diff --git a/modules/emacs/eshell/autoload/settings.el b/modules/emacs/eshell/autoload/settings.el index 39a838517..276f44b02 100644 --- a/modules/emacs/eshell/autoload/settings.el +++ b/modules/emacs/eshell/autoload/settings.el @@ -7,8 +7,11 @@ (signal 'wrong-number-of-arguments (list 'even (length aliases)))) (after! eshell (while aliases - (setf (alist-get (pop aliases) +eshell-aliases nil nil #'equal) - (list (pop aliases)))) + (let ((alias (pop aliases)) + (command (pop aliases))) + (if-let* ((oldval (assoc alias +eshell-aliases))) + (setcdr oldval (list command)) + (push (list alias command) +eshell-aliases)))) (when (boundp 'eshell-command-aliases-list) (if +eshell--default-aliases (setq eshell-command-aliases-list diff --git a/modules/feature/eval/autoload/repl.el b/modules/feature/eval/autoload/repl.el index dcd8df1a1..dd1cf1039 100644 --- a/modules/feature/eval/autoload/repl.el +++ b/modules/feature/eval/autoload/repl.el @@ -1,20 +1,21 @@ ;;; feature/eval/autoload/repl.el -*- lexical-binding: t; -*- -(defvar +eval-repl-buffers-alist () +(defvar +eval-repl-buffers (make-hash-table :test 'equal) "The buffer of the last open repl.") (define-minor-mode +eval-repl-mode "A minor mode for REPL buffers.") (defun +eval--ensure-in-repl-buffer (&optional command same-window-p) - (setq +eval-repl-buffers-alist - (cl-remove-if-not #'buffer-live-p +eval-repl-buffers-alist - :key #'cdr)) + (maphash (lambda (key buffer) + (unless (buffer-live-p buffer) + (remhash key +eval-repl-buffers))) + +eval-repl-buffers) (let* ((project-root (doom-project-root 'nocache)) (key (cons major-mode project-root)) - (buffer (cdr (assoc key +eval-repl-buffers-alist)))) - (unless (and (bufferp buffer) - (eq buffer (current-buffer))) + (buffer (gethash key +eval-repl-buffers))) + (cl-check-type buffer (or buffer null)) + (unless (eq buffer (current-buffer)) (funcall (if same-window-p #'switch-to-buffer #'pop-to-buffer) (if (buffer-live-p buffer) buffer @@ -26,8 +27,7 @@ (unless (bufferp buffer) (error "REPL command didn't return a buffer")) (with-current-buffer buffer (+eval-repl-mode +1)) - (setf (alist-get key +eval-repl-buffers-alist nil nil #'equal) - buffer) + (puthash key buffer +eval-repl-buffers) buffer))) (with-current-buffer buffer (goto-char (if (and (derived-mode-p 'comint-mode) From b077b2f6a6cba0b5531776b7084dcc00a181a416 Mon Sep 17 00:00:00 2001 From: Henrik Lissner Date: Sun, 9 Sep 2018 17:01:31 -0400 Subject: [PATCH 5/7] Use -y flag instead of piping yes to doom compile --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index da351bd35..4515b60a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,3 +18,4 @@ env: script: - bin/doom version - bin/doom -d test + - bin/doom -y compile From 8f5d822363503c9340e6ed2c74ea955e12b50ad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Sun, 9 Sep 2018 21:11:45 +0100 Subject: [PATCH 6/7] make compile-core: fix warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is easier to spot real problems if the code is warning-free. Replacing `gensym` with `make-symbol` is an idea taken from here: https://github.com/abo-abo/org-mode/commit/b44c08dd455236a63ae1da07cb30639d2679ba92 In defer-until!: core-lib.el:150:19:Warning: function ‘gensym’ from cl package called at runtime In add-transient-hook!: core-lib.el:216:16:Warning: function ‘gensym’ from cl package called at runtime In toplevel form: autoload/message.el:35:1:Warning: Unused lexical variable ‘spec’ In toplevel form: autoload/line-numbers.el:31:1:Warning: defcustom for ‘display-line-numbers-type’ fails to specify containing group autoload/line-numbers.el:31:1:Warning: defcustom for ‘display-line-numbers-type’ fails to specify containing group autoload/line-numbers.el:39:1:Warning: defcustom for ‘display-line-numbers-grow-only’ fails to specify containing group autoload/line-numbers.el:39:1:Warning: defcustom for ‘display-line-numbers-grow-only’ fails to specify containing group autoload/line-numbers.el:44:1:Warning: defcustom for ‘display-line-numbers-width-start’ fails to specify containing group autoload/line-numbers.el:44:1:Warning: defcustom for ‘display-line-numbers-width-start’ fails to specify containing group In toplevel form: cli/autoloads.el:137:1:Warning: Unused lexical variable ‘type’ Preserve name of unused lexical var _type Makes it obvious what is stored there. --- core/autoload/line-numbers.el | 3 +++ core/autoload/message.el | 2 +- core/cli/autoloads.el | 2 +- core/core-lib.el | 4 ++-- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/autoload/line-numbers.el b/core/autoload/line-numbers.el index 6ea35584d..a02a7b899 100644 --- a/core/autoload/line-numbers.el +++ b/core/autoload/line-numbers.el @@ -27,6 +27,9 @@ Lisp programs can disable display of a line number of a particular buffer line by putting the display-line-numbers-disable text property or overlay property on the first visible character of that line.") +(defgroup display-line-numbers nil "Display line number preferences" + :group 'emacs) + ;;;###autoload (defcustom display-line-numbers-type t "The default type of line numbers to use in `display-line-numbers-mode'. diff --git a/core/autoload/message.el b/core/autoload/message.el index 6421c839f..e7645d352 100644 --- a/core/autoload/message.el +++ b/core/autoload/message.el @@ -47,7 +47,7 @@ Otherwise, it maps colors to a term-color-* face." (propertize text 'face (append (get-text-property 0 'face text) - (let (spec) + (let (_) (cond ((>= code 40) `(:background ,(caddr (assq style doom-ansi-alist)))) ((>= code 30) diff --git a/core/cli/autoloads.el b/core/cli/autoloads.el index 4d427be58..e964bc5ae 100644 --- a/core/cli/autoloads.el +++ b/core/cli/autoloads.el @@ -192,7 +192,7 @@ even if it doesn't need reloading!" (push `(put ',name 'doom-module ',origin) forms)))) ((eq type 'defalias) - (cl-destructuring-bind (type name target &optional docstring) sexp + (cl-destructuring-bind (_type name target &optional docstring) sexp (let ((name (doom-unquote name)) (target (doom-unquote target))) (unless (and member-p diff --git a/core/core-lib.el b/core/core-lib.el index 0e764aede..e9275d53c 100644 --- a/core/core-lib.el +++ b/core/core-lib.el @@ -130,7 +130,7 @@ serve as a predicated alternative to `after!'." (declare (indent defun) (debug t)) `(if ,condition (progn ,@body) - ,(let ((fun (gensym "doom|delay-form-"))) + ,(let ((fun (make-symbol "doom|delay-form-"))) `(progn (fset ',fun (lambda (&rest args) (when ,(or condition t) @@ -196,7 +196,7 @@ advised)." (let ((append (if (eq (car forms) :after) (pop forms))) (fn (if (symbolp (car forms)) (intern (format "doom|transient-hook-%s" (pop forms))) - (gensym "doom|transient-hook-")))) + (make-symbol "doom|transient-hook-")))) `(progn (fset ',fn (lambda (&rest _) From d9a9243a62ba98055553b58b01e1b3e262329d2b Mon Sep 17 00:00:00 2001 From: Henrik Lissner Date: Sun, 9 Sep 2018 16:39:25 -0400 Subject: [PATCH 7/7] Remove unnecessary let block --- core/autoload/message.el | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/autoload/message.el b/core/autoload/message.el index e7645d352..b475cb5ad 100644 --- a/core/autoload/message.el +++ b/core/autoload/message.el @@ -47,12 +47,11 @@ Otherwise, it maps colors to a term-color-* face." (propertize text 'face (append (get-text-property 0 'face text) - (let (_) - (cond ((>= code 40) - `(:background ,(caddr (assq style doom-ansi-alist)))) - ((>= code 30) - `(:foreground ,(face-foreground (caddr (assq style doom-ansi-alist))))) - ((cddr (assq style doom-ansi-alist)))))))))) + (cond ((>= code 40) + `(:background ,(caddr (assq style doom-ansi-alist)))) + ((>= code 30) + `(:foreground ,(face-foreground (caddr (assq style doom-ansi-alist))))) + ((cddr (assq style doom-ansi-alist))))))))) ;;;###autoload (defmacro format! (message &rest args)