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

Pass checkdoc variables to subprocess #741

Closed
kbauer opened this issue Aug 30, 2015 · 9 comments
Closed

Pass checkdoc variables to subprocess #741

kbauer opened this issue Aug 30, 2015 · 9 comments
Assignees

Comments

@kbauer
Copy link

@kbauer kbauer commented Aug 30, 2015

Related to #53

Currently checkdoc as invoked by flycheck ignores customized variables. Relevant variables include all customization variables of checkdoc and at least sentence-end-double-space.

I currently solved this locally by a startup script setting flycheck-emacs-args to

(list "-Q" "--batch" "--eval"
      (pp-to-string
       `(setq sentence-end-double-space
              ',sentence-end-double-space))
      "--eval"
      (pp-to-string
       `(progn
          ,@(cl-loop for var being the symbols
                     if (and (custom-variable-p var)
                             (string-match-p "^checkdoc-"
                                             (symbol-name var)))
                     collect `(setq ,var ',(symbol-value var)))))))
@lunaryorn lunaryorn added the ready label Aug 31, 2015
@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Aug 31, 2015

@kbauer I don't see the need for this myself, because personally I use the default checkdoc settings, and encourage everyone else to do the same, as these settings represent the standard Emacs style for docstrings.

Closing as “wontfix” hence, because I won't implement this feature myself.

However, I'll happily merge pull requests or patches that add these variables to the checkdoc checker. Note, though, that I do not like your approach of blindly passing all checkdoc variables. I'd rather prefer if every variables was explicitly forwarded as an option to the checkdoc checker.

@lunaryorn lunaryorn closed this Aug 31, 2015
@lunaryorn lunaryorn added status: wontfix and removed ready labels Aug 31, 2015
@cpitclaudel
Copy link
Member

@cpitclaudel cpitclaudel commented Dec 20, 2015

@lunaryorn, I was looking at this and wondering: have we ever considered support for file-local variables as a way to handle this problem? This would be similar to how some checkers allow you to configure the warnings for a file by adding special comments (and I would consider it good practice to make such settings parts of a file, so that everyone in a project checks the file according to the same standards)

@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Dec 20, 2015

@cpitclaudel I'm not sure what you mean. Do you think the checkdoc syntax checker should read a local variables file explicitly?

@cpitclaudel
Copy link
Member

@cpitclaudel cpitclaudel commented Dec 20, 2015

Probably, yes; I was wondering what the checkdoc equivalent would be of (say) the following header (taken from a python file of mine):

#!/usr/bin/env python2
# -*- coding: utf-8 -*-
# pylint: disable=too-few-public-methods

and it occurred to me that we might be interested in supporting something like

;; Local Variables:
;; checkdoc-arguments-in-order-flag: nil
;; End:

for checkdoc.

Does that make sense?

@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Dec 25, 2015

@cpitclaudel We should definitely support this, but we should not try to parse directory variables manually in the checkdoc subprocess. We should rely on Emacs to handle directory variables for us in the main instance, and then just propagate all checkdoc variables from the current buffer down to the checkdoc sub-process.

@cpitclaudel
Copy link
Member

@cpitclaudel cpitclaudel commented Dec 25, 2015

@lunaryorn Yes, that sounds much better :) Let me make a small patch.

@bbatsov
Copy link
Contributor

@bbatsov bbatsov commented Apr 2, 2016

@kbauer I don't see the need for this myself, because personally I use the default checkdoc settings, and encourage everyone else to do the same, as these settings represent the standard Emacs style for docstrings.

Well, a lot of the Emacs code actually violates some of the defaults as they are a bit moronic (e.g. mention arguments in order). And some rules simply generate a ton of false positives (like the imperative checks). Btw, won't this be automatically solved if you simply copied any .dir-locals files you encounter to the temp directory you use to check the code in?

@cpitclaudel
Copy link
Member

@cpitclaudel cpitclaudel commented Apr 2, 2016

Btw, won't this be automatically solved if you simply copied any .dir-locals files you encounter to the temp directory you use to check the code in?

It's a bit tricky: what about unsafe variables, or variables that the user refused to apply when opening a file?

Emacs Lisp checkers already use source-inplace, so there should be no need to copy .dir-locals files.

cpitclaudel added a commit that referenced this issue Apr 2, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process).

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
cpitclaudel added a commit that referenced this issue Apr 2, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process).

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
@cpitclaudel cpitclaudel reopened this Apr 2, 2016
@cpitclaudel
Copy link
Member

@cpitclaudel cpitclaudel commented Apr 2, 2016

Reopening to match #937

cpitclaudel added a commit that referenced this issue Apr 4, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process).

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
cpitclaudel added a commit that referenced this issue Apr 4, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process).

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
cpitclaudel added a commit that referenced this issue Apr 4, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process). This list is kept explicitly,
because generating it requires browsing the checkdoc customization
group, which is only populated after loading checkdoc.

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
cpitclaudel added a commit that referenced this issue Apr 4, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process). This list is kept explicitly,
because generating it requires browsing the checkdoc customization
group, which is only populated after loading checkdoc. A new test
ensures that the list is up-to-date.

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
cpitclaudel added a commit that referenced this issue Apr 4, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process). This list is kept explicitly,
because generating it requires browsing the checkdoc customization
group, which is only populated after loading checkdoc. A new test
ensures that the list is up-to-date.

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
cpitclaudel added a commit that referenced this issue Apr 4, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process). This list is kept explicitly,
because generating it requires browsing the checkdoc customization
group, which is only populated after loading checkdoc. A new test
ensures that the list is up-to-date.

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
cpitclaudel added a commit that referenced this issue Apr 4, 2016
‘flycheck-emacs-lisp-checkdoc-variables’ contains all variables that are
passed to the inferior Emacs process running checkdoc (but only if they
are already bound in the parent process). This list is kept explicitly,
because generating it requires browsing the checkdoc customization
group, which is only populated after loading checkdoc. A new test
ensures that the list is up-to-date.

The main use of this is allowing for file- and directory-local checkdoc
settings.

Closes GH-741.
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
4 participants