Emacs-D-Mode-Maintainers / Emacs-D-Mode

An Emacs mode for D code.
GNU General Public License v3.0
84 stars 22 forks source link

d-mode.el contains apparently-broken code to update auto-mode-alist by default #88

Closed ssb22 closed 6 years ago

ssb22 commented 6 years ago

d-mode.el's Usage comment says to put into my init file (autoload 'd-mode "d-mode" "Major mode for editing D code." t) and (add-to-list 'auto-mode-alist '("\\.d[i]?\\'" . d-mode)), but d-mode.el also contains the line ;;;###autoload (add-to-list 'auto-mode-alist '("\\.d[i]?\\'" . d-mode)), which does nothing on my Emacs 25.3.1. The following "Custom variables" section has a newline after the ;;;###autoload so I suspect a newline might have been meant before the (add-to-list 'auto-mode-alist as well, to make the auto-mode-alist update happen by default, and then it won't be necessary to tell the user to put that second line into their init file in the Usage section.

CyberShadow commented 6 years ago

Here is the generated autoloads file from the latest version:

;;; d-mode-autoloads.el --- automatically extracted autoloads
;;
;;; Code:
(add-to-list 'load-path (directory-file-name (or (file-name-directory #$) (car load-path))))

;;;### (autoloads nil "d-mode" "d-mode.el" (23273 35317 204642 870000))
;;; Generated autoloads from d-mode.el
 (add-to-list 'auto-mode-alist '("\\.d[i]?\\'" . d-mode))

(defvar d-mode-hook nil "\
*Hook called by `d-mode'.")

(custom-autoload 'd-mode-hook "d-mode" t)

(autoload 'd-mode "d-mode" "\
Major mode for editing code written in the D Programming Language.

See http://dlang.org for more information about the D language.

The hook `c-mode-common-hook' is run with no args at mode
initialization, then `d-mode-hook'.

Key bindings:
\\{d-mode-map}

\(fn)" t nil)

;;;***

;; Local Variables:
;; version-control: never
;; no-byte-compile: t
;; no-update-autoloads: t
;; End:
;;; d-mode-autoloads.el ends here

I don't see a problem here.

Usage comment says to put into my init file

If you're not using package.el, which would otherwise do it for you. I guess the comment could be updated...

ssb22 commented 6 years ago

Yes the generated autoloads are fine, but the problem is when running as directed in the Usage comment i.e. not via packages.el. We can support that use-case easily enough by adding a newline as above. Separately, it might also be a good idea to expand the Usage comment to mention packages.el. Thanks.

CyberShadow commented 6 years ago

Sorry, I'm having some trouble understanding what this issue is about.

ssb22 commented 6 years ago

OK pull request submitted.

CyberShadow commented 6 years ago

Sorry, I can't merge that without an understanding of what the change is trying to accomplish. A test case would be OK too.

ssb22 commented 6 years ago

OK, so starting from emacs -q :

Press C-x b RET to switch to buffer scratch

Type (load "~/path/to/d-mode.el")

Press C-x C-e to evaluate

Press C-x C-f to find file, enter any file ending .d

D mode does not activate, because it's not in auto-mode-alist.

The Usage comment says you have to add it to auto-mode-alist in a separate operation after you have loaded d-mode.el. But there is code in there to add it automatically, it's just that it doesn't work because it has ;;;###autoload before it (with no newline), meaning emacs ignores it when parsing d-mode.el in this way. Once the newline is added, this problem goes away and it is no longer necessary to add it to auto-mode-alist in a separate operation after you have loaded d-mode.el.

CyberShadow commented 6 years ago

Got it, the problem is that because ;;;###autoload (add-to-list 'auto-mode-alist '("\\.d[i]?\\'" . d-mode)) is a comment, it is evaluated only during autoload, and not when it's loaded regularly.