vermiculus / magithub

**DEPRECATED - please use Forge instead!** -- Magit-based interfaces to GitHub
GNU General Public License v3.0
579 stars 63 forks source link

Pass a feature to eval-after-load instead of a string #275

Closed cpitclaudel closed 6 years ago

cpitclaudel commented 6 years ago

Thanks for this package! It looks great. I'm trying to run it, but it breaks my Emacs startup.

(magit-define-popup-action 'magit-dispatch-popup 72 "Magithub" (function magithub-dispatch-popup) 33)
  (progn (magit-define-popup-action 'magit-dispatch-popup 72 "Magithub" (function magithub-dispatch-popup) 33) (define-key magit-status-mode-map "H" (function magithub-dispatch-popup)))
  (lambda nil (progn (magit-define-popup-action 'magit-dispatch-popup 72 "Magithub" (function magithub-dispatch-popup) 33) (define-key magit-status-mode-map "H" (function magithub-dispatch-popup))))()
  funcall((lambda nil (progn (magit-define-popup-action 'magit-dispatch-popup 72 "Magithub" (function magithub-dispatch-popup) 33) (define-key magit-status-mode-map "H" (function magithub-dispatch-popup)))))
  mapc(funcall ((lambda nil (progn (magit-define-popup-action 'magit-dispatch-popup 72 "Magithub" (function magithub-dispatch-popup) 33) (define-key magit-status-mode-map "H" (function magithub-dispatch-popup))))))
  do-after-load-evaluation("/home/clement/.emacs.d/init/mode-specific/magit.el")
  load-with-code-conversion("/home/clement/.emacs.d/init/mode-specific/magit.el" "/home/clement/.emacs.d/init/mode-specific/magit.el" nil nil)
  load("/home/clement/.emacs.d/init/mode-specific/magit.el")
  (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path)))
  (let* ((start-time (current-time))) (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path))))
  (if (file-exists-p path) (let* ((start-time (current-time))) (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path)))) (warn "[%s] skipped" relative-path))
  (let ((path (expand-file-name relative-path ~/init-dir))) (if (file-exists-p path) (let* ((start-time (current-time))) (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path)))) (warn "[%s] skipped" relative-path)))
  ~/load-file("/home/clement/.emacs.d/init/mode-specific/magit.el")
  mapc(~/load-file ("/home/clement/.emacs.d/init/mode-specific/adaptive-wrap.el" "/home/clement/.emacs.d/init/mode-specific/agda.el" "/home/clement/.emacs.d/init/mode-specific/checkdoc.el" "/home/clement/.emacs.d/init/mode-specific/company.el" "/home/clement/.emacs.d/init/mode-specific/coq.el" "/home/clement/.emacs.d/init/mode-specific/csharp.el" "/home/clement/.emacs.d/init/mode-specific/dafny.el" "/home/clement/.emacs.d/init/mode-specific/diminish.el" "/home/clement/.emacs.d/init/mode-specific/expand-region.el" "/home/clement/.emacs.d/init/mode-specific/flycheck.el" "/home/clement/.emacs.d/init/mode-specific/gpg.el" "/home/clement/.emacs.d/init/mode-specific/gud.el" "/home/clement/.emacs.d/init/mode-specific/haskell.el" "/home/clement/.emacs.d/init/mode-specific/ibuffer.el" "/home/clement/.emacs.d/init/mode-specific/ido.el" "/home/clement/.emacs.d/init/mode-specific/java.el" "/home/clement/.emacs.d/init/mode-specific/js.el" "/home/clement/.emacs.d/init/mode-specific/latex.el" "/home/clement/.emacs.d/init/mode-specific/ledger.el" "/home/clement/.emacs.d/init/mode-specific/lisp.el" "/home/clement/.emacs.d/init/mode-specific/magit.el" "/home/clement/.emacs.d/init/mode-specific/markdown.el" "/home/clement/.emacs.d/init/mode-specific/mu4e.el" "/home/clement/.emacs.d/init/mode-specific/nameless.el" "/home/clement/.emacs.d/init/mode-specific/ocaml.el" "/home/clement/.emacs.d/init/mode-specific/org.el" "/home/clement/.emacs.d/init/mode-specific/python.el" "/home/clement/.emacs.d/init/mode-specific/rust.el" "/home/clement/.emacs.d/init/mode-specific/writeroom.el"))
  eval-buffer(#<buffer  *load*-418945> nil "/home/clement/.emacs.d/init/mode-specific.el" nil t)  ; Reading at buffer position 6312
  load-with-code-conversion("/home/clement/.emacs.d/init/mode-specific.el" "/home/clement/.emacs.d/init/mode-specific.el" nil nil)
  load("/home/clement/.emacs.d/init/mode-specific.el")
  (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path)))
  (let* ((start-time (current-time))) (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path))))
  (if (file-exists-p path) (let* ((start-time (current-time))) (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path)))) (warn "[%s] skipped" relative-path))
  (let ((path (expand-file-name relative-path ~/init-dir))) (if (file-exists-p path) (let* ((start-time (current-time))) (prog1 (load path) (message "%.4fs [%s]" (float-time (time-since start-time)) (file-name-nondirectory relative-path)))) (warn "[%s] skipped" relative-path)))
  ~/load-file("mode-specific.el")
  mapc(~/load-file ("compatibility.el" "bug-fixes.el" "custom.el" "package.el" "general.el" "fonts.el" "defuns.el" "mode-specific.el" "os-specific.el" "keybindings.el" "autoloads.el" "properties.el" "local.el"))
  eval-buffer(#<buffer  *load*> nil "/home/clement/.emacs.d/init.el" nil t)  ; Reading at buffer position 1623
  load-with-code-conversion("/home/clement/.emacs.d/init.el" "/home/clement/.emacs.d/init.el" t t)
  load("/home/clement/.emacs.d/init" t t)
  #f(compiled-function () #<bytecode>)()
  command-line()
  normal-top-level()

The reason is that I collect my magit setttings in a file called magit.el. That file isn't in my load path, so it doesn't create a conflict and it doesn't cause issues when requiring magit.

It does, however, conflict with the autoloads in magithub, including this one:

(eval-after-load "magit"
  '(progn
     (magit-define-popup-action 'magit-dispatch-popup
       ?H "Magithub" #'magithub-dispatch-popup ?!)
     (define-key magit-status-mode-map
"H" #'magithub-dispatch-popup)))

This part of the docs is relevant:

If FILE is a string, it may be either an absolute or a relative file
name, and may have an extension (e.g. ".el") or may lack one, and
additionally may or may not have an extension denoting a compressed
format (e.g. ".gz").
…
When FILE lacks an extension, a file name with any extension will trigger
evaluation. 
…
Alternatively, FILE can be a feature (i.e. a symbol), in which case FORM
is evaluated at the end of any file that ‘provide’s this feature.
If the feature is provided when evaluating code not associated with a
file, FORM is evaluated immediately after the provide statement.

IOW, (eval-after-load "magit") runs its body any time a file called magit.el is loaded, whereas (eval-after-load 'magit) runs its body only after (provide 'magit). I think we want the latter, right?

cpitclaudel commented 6 years ago

The CI failure looks like a fluke; the log says this:

emacs --version
make: *** [setup-CI] Illegal instruction (core dumped)
vermiculus commented 6 years ago

The CI failure looks like a fluke; the log says this:

emacs --version
make: *** [setup-CI] Illegal instruction (core dumped)

Yeah, it's been doing that :-( I'm not sure what's causing it, but I'm not the only one; I remember seeing a recent issue about this in other repositories.

For now, I've just been restarting the builds; it seems to be random. Looks like the build is succeeding now, though :-)

vermiculus commented 6 years ago

Merged, thanks!!