Skip to content
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

Should we put the emacs lisp checkdoc buffer in emacs-lisp mode? #833

Closed
cpitclaudel opened this issue Dec 31, 2015 · 5 comments
Closed

Should we put the emacs lisp checkdoc buffer in emacs-lisp mode? #833

cpitclaudel opened this issue Dec 31, 2015 · 5 comments
Assignees

Comments

@cpitclaudel
Copy link
Member

@cpitclaudel cpitclaudel commented Dec 31, 2015

The following snippet causes our checkdoc checker to fail:

(defun confusing () ?{)
-*- mode: compilation; default-directory: "~/.emacs.d/lisp/company-coq/" -*-
Compilation started at Thu Dec 31 18:15:38

/usr/local/bin/emacs -Q --batch --eval \(progn\ \(defvar\ jka-compr-inhibit\)\ \(unwind-protect\ \(let\ \(\(jka-compr-inhibit\ t\)\)\ \(when\ \(equal\ \(car\ command-line-args-left\)\ \"--\"\)\ \(setq\ command-line-args-left\ \(cdr\ command-line-args-left\)\)\)\ \(require\ \'checkdoc\)\ \(let\ \(\(source\ \(car\ command-line-args-left\)\)\ \(process-default-directory\ default-directory\)\)\ \(with-temp-buffer\ \(insert-file-contents\ source\ \'visit\)\ \(setq\ buffer-file-name\ source\)\ \(setq\ default-directory\ process-default-directory\)\ \(with-demoted-errors\ \"Error\ in\ checkdoc\:\ \%S\"\ \(checkdoc-current-buffer\ t\)\ \(with-current-buffer\ checkdoc-diagnostic-buffer\ \(princ\ \(buffer-substring-no-properties\ \(point-min\)\ \(point-max\)\)\)\ \(kill-buffer\)\)\)\)\)\)\ \(setq\ command-line-args-left\ nil\)\)\) -- /home/clement/.emacs.d/lisp/company-coq/company-coq.el
Error in checkdoc: (scan-error "Unbalanced parentheses" 1 25)

Compilation finished at Thu Dec 31 18:15:38

Changing the temp buffer to emacs-lisp-mode solves the problem; but that might be a bit drastic. @lunaryorn, what do you think?

@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Jan 1, 2016

@cpitclaudel Thanks. I'm not sure I understand the issue.

As far as I can see the code is syntactically wrong, isn't it? But then, why did the checkdoc checker even run on it? Actually the emacs-lisp checker should run first, find the syntax error and stop further processing…

@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Jan 1, 2016

Ah forget what I said. It's valid code actually. Seems that checkdoc relies on the syntax table to scan the buffer. In this case we should really put the temp buffer into emacs-lisp-mode.

@cpitclaudel
Copy link
Member Author

@cpitclaudel cpitclaudel commented Jan 1, 2016

Can anything bad happen from using (emacs-lisp-mode)? I guess not much, since we're in emacs -Q anyway.

@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Jan 2, 2016

@cpitclaudel I don't think so. We should wrap with in delay-mode-hooks, i.e.

(delay-mode-hooks (emacs-lisp-mode))
(setq delayed-mode-hooks nil)

This prevents emacs-lisp-mode-hook and its parents from running. Shouldn't be needed in emacs -Q, but just in case… ☺️

Would you mind to apply that change to Flycheck? Probably also with a small test case that uses the above code example to make sure that we don't break this again?

@cpitclaudel
Copy link
Member Author

@cpitclaudel cpitclaudel commented Mar 1, 2016

Looks like with-syntax-table wasn't enough :/ The following example produces spurious warnings as well:

(defconst a
  ;; comment "a"
  "Docs for a.")

I'll prepare a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants