Skip to content

Commit

Permalink
fix(cli): module cli.el loader
Browse files Browse the repository at this point in the history
$DOOMDIR/init.el had to be loaded earlier, so we could read the active
module list. This indirectly fixes an issue where users' literate
configs weren't being tangled on 'doom sync'.

Fix: #6479
  • Loading branch information
hlissner committed Jun 20, 2022
1 parent fe85093 commit 19ce459
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions bin/doom
Expand Up @@ -105,6 +105,10 @@

(require 'core-cli (expand-file-name "core/core-cli" user-emacs-directory))

;; Load $DOOMDIR/init.el, to read the user's `doom!' block, and so users can
;; customize things early, if they like.
(load! doom-module-init-file doom-private-dir t)


;;
;;; Entry point
Expand Down Expand Up @@ -218,9 +222,6 @@ SEE ALSO:
(when pager
(setenv "DOOMPAGER" pager))
(exit! :restart))
;; Load $DOOMDIR/init.el, so users can customize things, if they like.
(doom-log "Loading $DOOMDIR/init.el")
(load! doom-module-init-file doom-private-dir t)
;; Load extra files and forms, as per given options.
(dolist (file loads)
(load (doom-path (cdr file))
Expand Down Expand Up @@ -288,10 +289,9 @@ SEE ALSO:
(let ((cli-file "cli"))
(defgroup! "Module commands"
(dolist (key (hash-table-keys doom-modules))
(when-let* ((path (plist-get (gethash key doom-modules) path))
(path (car (doom-glob path cli-file))))
(when-let (path (plist-get (gethash key doom-modules) :path))
(defgroup! :prefix (format "+%s" (cdr key))
(defautoload! () path)))))
(load! cli-file path t)))))

(doom-log "Loading $DOOMDIR/cli.el")
(load! cli-file doom-private-dir t))))
Expand Down

12 comments on commit 19ce459

@catap
Copy link
Contributor

@catap catap commented on 19ce459 Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hlissner it has one significant side effect.

Let assume that someone like me would like to keep SSH_AUTH_SOCK inside env file. By following a suggestion from #2434 I had in my init.el lines like:

(when (boundp 'doom-env-whitelist)
  (add-to-list 'doom-env-whitelist "^SSH_"))

Since this changes a condition always false that makes such hack uselsees. The right one after this commit is:

(when noninteractive
  (setq doom-env-allow '("^SSH_")))

but it quite ugly :(

@hlissner
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@catap Actually, you just need an after! block:

(after! core-cli-env
  (add-to-list 'doom-env-whitelist "^SSH_"))

That said, some warning: later next month, in preparation for moving Doom's modules into another repo, I will rename core/core-*.el to lisp/doom-*.el, and all the features will be renamed from core-* to doom-*. E.g. core-cli-env => doom-cli-env. Keep that in mind when that breaks.

@catap
Copy link
Contributor

@catap catap commented on 19ce459 Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hlissner thanks, but it should be:

(after! core-cli-env
  (add-to-list 'doom-env-allow "^SSH_"))

you have renamed doom-env-whitelist to doom-env-allow

@catap
Copy link
Contributor

@catap catap commented on 19ce459 Aug 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for records: after b9933e6 the code should look likes:

(after! doom-cli-env
  (add-to-list 'doom-env-allow "^SSH_"))

@hungnh17

This comment was marked as resolved.

@catap

This comment was marked as resolved.

@hungnh17

This comment was marked as resolved.

@hungnh17

This comment was marked as resolved.

@jeetelongname

This comment was marked as resolved.

@bettafish15

This comment was marked as resolved.

@catap

This comment was marked as resolved.

@hungnh17

This comment was marked as resolved.

Please sign in to comment.