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

Spellcheck of docstrings #7

Closed
DamienCassou opened this issue Jan 18, 2020 · 15 comments
Closed

Spellcheck of docstrings #7

DamienCassou opened this issue Jan 18, 2020 · 15 comments

Comments

@DamienCassou
Copy link
Contributor

Thanks to this patch, Emacs 27 can now spellcheck docstrings when run in batch mode. This requires setting checkdoc-spellcheck-documentation-flag to a non-nil value as nil is the default.

Would you consider changing the value of this variable or letting the user change it?

@alphapapa
Copy link
Owner

I'd be glad to support that. I don't mind enabling it by default, although I guess I'll have to gate that code on the Emacs version, right?

@alphapapa
Copy link
Owner

I see, only the batch part is new.

I think that should do it. I don't have Emacs 27 installed, so please let me know if it works properly. :)

@DamienCassou
Copy link
Contributor Author

Thank you

@alphapapa
Copy link
Owner

@DamienCassou Hmm, I'm not sure if this is a good idea to enable by default yet. For example: https://github.com/alphapapa/plz.el/runs/420621309?check_suite_focus=true#step:5:10 Do you have any ideas?

@alphapapa alphapapa reopened this Feb 1, 2020
@DamienCassou
Copy link
Contributor Author

As with all linters, you will get good feedback and some less good one. I suggest adding the words you like to the file's local dictionary. This can be done with ispell-word and then A which adds a line like this at the end of the file:

; LocalWords:  undecoded struct unparsed

You can also change the words:

  • undecoded => raw
  • struct => structure
  • unparsed => raw (or header text)

@tastytea
Copy link

aspell uses the language from LC_MESSAGES for checking. If it is anything other than en_*, most packages will get a lot of errors.

@alphapapa
Copy link
Owner

@tastytea Is aspell always the backend that's used? Do users in other locales already have to change that when running checkdoc within Emacs? @DamienCassou Do you have any insight into that issue?

@tastytea
Copy link

The manual mentions “Hunspell, Aspell, Ispell or Enchant“ as possible backends. hunspell also uses LC_MESSAGES, ispell uses DICTIONARY and enchant is a frontend for “aspell/pspell, myspell/hunspell, ispell, uspell, hspell, voikko, zemberek, and Apple Spell (macOS only).”.

Users in other locales have to set ispell-local-dictionary to "english" when running checkdoc within Emacs.

While programmers are likely to have the right dictionary already set up, local calls to makem.sh with --sandbox will select the wrong dictionary.

CI images often have no locale installed and set LC_MESSAGES to C or nothing. When I replicate that locally, I'm getting the same errors, but that might be because I have the dictionaries for “de” and “en” installed and “de” is alphabetically the first.

@DamienCassou
Copy link
Contributor Author

@tastytea Is aspell always the backend that's used?

no. The user can choose through ispell-program-name.

Do users in other locales already have to change that when running checkdoc within Emacs? @DamienCassou Do you have any insight into that issue?

I've never had any issue. I always write code in English but I regularly change the dictionary for written text. Maybe makel could set english as default because this is what most elisp is written in and let users change that.

@meedstrom
Copy link

meedstrom commented Aug 26, 2021

The checkdoc lint failed on my Github Action, no spell program seems to be installed with the default test.yml.

Starting new Ispell process ispell with default dictionary... \ 
No spellchecker installed: check the variable ‘ispell-program-name’
ERROR (2021-08-26 13:37:37): Linting checkdoc failed.

For now, I set checkdoc-spellcheck-documentation-flag to nil in the makem.sh source. Just checking, but was there a way to do that on the command line or in test.yml?

For what it's worth, the checkdoc also fails locally on my machine, even though it seems to discover aspell:

[16:14] $ ./makem.sh --install-deps --install-linters -ssandbox lint-checkdoc
Starting new Ispell process /home/kept/guix-profiles/emacs/emacs/bin/aspell with default dictionary... \ 
Starting new Ispell process /home/kept/guix-profiles/emacs/emacs/bin/aspell with default dictionary...done
No spellchecker installed: check the variable ‘ispell-program-name’
ERROR (2021-08-26 16:14:12): Linting checkdoc failed.
LOG (2021-08-26 16:14:12): Finished with 1 errors.

@alphapapa
Copy link
Owner

The checkdoc lint failed on my Github Action, no spell program seems to be installed with the default test.yml.

Yes, and it used to work, and it stopped working without any changes in this repo, so unfortunately something changed in one of the dependencies, and I haven't bothered to go looking for what got broken yet. Probably one of the Nix Emacs things stopped installing aspell by default, or something like that.

@jscheid
Copy link

jscheid commented May 31, 2022

I've run into an issue with LocalWords when there are multiple files being linted. checkdoc-ispell-init will end up reading them only for the first file and reuse that config for subsequent files. In my case that was breaking things, the first file happened to have no LocalWords at all and the LocalWords defined in the second file never got read, leading to lots of false positives.

I'm working around it like so in elisp-checkdoc-file:

(advice-add 'checkdoc-file :before (lambda (_) (ispell-kill-ispell t t)))

@noctuid
Copy link

noctuid commented Jun 5, 2022

Can this be opt-in (with dir-locals), or if not, can there be a simple way to opt out?

@alphapapa
Copy link
Owner

See also #39.

@alphapapa
Copy link
Owner

FYI to those who are interested: 4bdfd81 should make this much easier for custom words. Thanks to @josephmturner.

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

No branches or pull requests

6 participants