New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the right syntax table for Emacs Lisp checkdoc #845

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@cpitclaudel
Member

cpitclaudel commented Jan 5, 2016

Looks like with-syntax-table is enough :)
Fixes #833

@@ -3844,7 +3851,7 @@ See https://github.com/flycheck/flycheck/issues/531 and Emacs bug #19206"))
"the function ‘dummy-package-foo’ is not known to be defined.")
:checker emacs-lisp)))
(flycheck-ert-def-checker-test emacs-lisp emacs-lisp sytnax-error
(flycheck-ert-def-checker-test emacs-lisp emacs-lisp syntax-error

This comment has been minimized.

@lunaryorn

lunaryorn Jan 5, 2016

Contributor

Ugh 😊 Thanks for fixing that :)

(checkdoc-current-buffer t)
;; Checkdoc needs the right syntax table
;; See https://github.com/flycheck/flycheck/issues/833
(with-syntax-table emacs-lisp-mode-syntax-table

This comment has been minimized.

@lunaryorn

lunaryorn Jan 5, 2016

Contributor

Don't we need an appropriate require for this syntax table?

This comment has been minimized.

@cpitclaudel

cpitclaudel Jan 6, 2016

Member

Indeed, it would be better to add it. I updated the PR.
The file providing that table changed between 24 and 25 (lisp-mode to elisp-mode). The new code loads elisp-mode and falls back to list-mode, but we could also use a version-based check.

@cpitclaudel cpitclaudel force-pushed the fix-833 branch from a04c039 to 099fa35 Jan 6, 2016

@@ -6224,6 +6224,9 @@ See Info Node `(elisp)Byte Compilation'."
(defconst flycheck-emacs-lisp-checkdoc-form
(flycheck-prepare-emacs-lisp-form
(require 'checkdoc)
;; elisp-mode provides the elisp syntax table
(unless (require 'elisp-mode nil t)

This comment has been minimized.

@lunaryorn

lunaryorn Jan 6, 2016

Contributor

@cpitclaudel I prefer to use symbols for non-nil arguments, i.e. (require … … 'no-error), because that's easier to read than the meaningless t :)

I'll change that before merging :)

@lunaryorn lunaryorn removed the in progress label Jan 6, 2016

@lunaryorn

This comment has been minimized.

Contributor

lunaryorn commented Jan 6, 2016

@cpitclaudel Merged with some slight modifications, see 7dd0c87. I deleted the branch.

Thank you very much for discovering and fixing this issue! 👍

@lunaryorn lunaryorn added reviewed and removed status: blocked labels Jan 6, 2016

@lunaryorn lunaryorn deleted the fix-833 branch Jan 6, 2016

@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented Jan 6, 2016

Beautiful, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment