Add a spell checker for libcore/libstd documentation #74697
Comments
jyn514
added
C-enhancement
|
Maybe we should also ping the author here @drahnr. He may have some ideas? |
|
@pickfire @jyn514 glad to hear there is interest in using The error you see is very most likely caused by an older version which had issues parsing args properly, note that I plan to release If there are specific issues I would be very happy to get things working for you :) |
@drahnr What do you think? |
@pickfire As @euclio already stated in #74141 (comment) - it should not be required anymore since |
|
It doesn't work and probably displayed the wrong error. |
|
@pickfire could you please open an issue with information on how to reproduce and commit sha / release so I can test / reproduce the issue? |
|
|
|
@drahnr I checked, a lot better but still not sufficient yet. Looks like UTF-8 handling still needs some tweak to make it work without panicking. |
|
Found the root cause, |
|
I tried running it: it showed no output for ~three seconds then exited. With -vvvv I get the following output: It looks like it's skipping almost all the directories? And not running checks on the ones that it did find, because the /usr/share/* paths weren't found? I would expect both of those to be warnings, not DEBUG logging. I installed However these warnings aren't particularly helpful - |
I am missing the exit code above and did it show anything besides the displayed print? Unfortunately, a cargo manifest does not contain all products, i.e. Adding a debug message and exit early seems to be reasonable if no dictionaries are configured / found, but that's not really what's happening here I think.
Frankly I never tested it without the presence of any dictionary files, I hoped that the configuration section would be sufficient to avoid this case entierly, but I was wrong :) https://github.com/drahnr/cargo-spellcheck#configuration
I would expect those to be contained within |
|
@jyn514 I think it would be understandable if we add all non-words like |
|
If this is not sufficient, I'd be open to define a regex set in the config for the |
I'd be in favour of pinging the docs team. |
Not all of the warnings were for rust types, though. It was flagging various other things like None of those should be in backticks, and it seems silly to whitelist each individually.
Ok, I think that should be fixed before we go further so that new contributors don't have differences between their setup and CI (or at least, know that the differences exist).
The exit code was 0. It did not show any other output.
I don't really understand this paragraph. What does this have to do with the message 'failed to complete'? Is the whole crate being skipped or just a single file? |
There isn't really a docs team: rust-lang/team#298. But I can ping @rust-lang/docs anyway in case people still get notifications. |
Nothing is skipped, it's just a bit odd behaviour that
Fully agree, but these are tricky - I think this would mandate some extra quirk code.
Could you open a new issue with the relevant information so I can reproduce this within an OCI container? |
|
|
|
@drahnr a point of good news since I know this has mostly been feature requests so far: this caught a typo in rustfmt :) |
|
|
|
@drahnr This sharing the crate to this week in rust and also put a call for partition in this week in rust to ask for feedback. Maybe you can also write a blog about it. |
|
@rustbot label: +E-help-wanted |
rustbot
added
the
E-help-wanted
|
Just gave it a try on the libstd. As far as I can see, it's mostly missing words or missing backticks. The full output is here: |
^ this looks like a bug, it doesn't recognize that code tags are the same as backticks. |
Could you file an issue, I wasn't aware this is legal syntax and so far nobody else reported an issue for this. I'll fix it ASAP. |
|
Also I'm very late but I just saw
You can get the full products with |
This is a bug somewhere; not sure whether it's in libstd, spellcheck, or some interaction with @GuillaumeGomez's locale. |
|
My locales: $ locale
LANG=en_US.UTF-8
LANGUAGE=
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC=fr_FR.UTF-8
LC_TIME=fr_FR.UTF-8
LC_COLLATE="en_US.UTF-8"
LC_MONETARY=fr_FR.UTF-8
LC_MESSAGES="en_US.UTF-8"
LC_PAPER=fr_FR.UTF-8
LC_NAME=fr_FR.UTF-8
LC_ADDRESS=fr_FR.UTF-8
LC_TELEPHONE=fr_FR.UTF-8
LC_MEASUREMENT=fr_FR.UTF-8
LC_IDENTIFICATION=fr_FR.UTF-8
LC_ALL=So I think the problem comes from Also to be noted: |
|
There is no locale interaction, it assumes UTF-8 for files and assumes en_US locale by default, which is used exclusively to determine the affix and dictionary files for hunspell. The prints are all UTF-8 as well, so that's the only side I think could mess things up. Speed is always improvable, the biggest boost would be to store the constructed tokenizer info, there was an issue drahnr/cargo-spellcheck#57 but the majority of this is marked as done now, since the remaining potential is mostly in the dependencies Please file an issue, if you think there could be an issue or it hampers usability. If something doesn't hit the issue tracker it doesn't exist for me. So far it has worked ok for small to large code bases. |
I implemented a workaround and released v0.11.2 . I came across another issue, backlogged as drahnr/cargo-spellcheck#263 |
From #74141:
It would be great to have a tool that automated spell-checking for the docs. @pickfire suggested
cargo spellcheckwhich had some trouble with bootstrap. Maybe we could somehow modify it to work with bootstrap so it runs as part oftidy?The text was updated successfully, but these errors were encountered: