cspell icon indicating copy to clipboard operation
cspell copied to clipboard

Refactoring docs + spring cleaning + some rewriting

Open Zamiell opened this issue 3 years ago • 14 comments

Closes #2758.

I originally just wanted to make a very small pull request to the docs, but I got sucked in by several things that were bothering me, and I ended up spending the day going through as much as I could and cleaning it up.

It all started when I wanted to change some content, but realized that I would have to change it in multiple places in order to keep the docs consistent. This kind of thing leads to bugs and/or inconsistencies, so I did a bit of refactoring to try and consolidate information to a single place.

Secondly, I reorganized the ordering of the doc pages to make more sense, and to cluster extraneous info into a "More Info" section, which makes it much more clean. Also combined the "About" page with the index page.

I can go into more detail about any particular change as to why I did it.

I think that configuration documentation specifically should always live in a JSDoc comment on CSpellSettingsDef.ts, so I migrated several pieces of information there. That way, if a setting is deleted, the docs don't have to be manually updated.

cspell-types changes

I ran npm build-docs because I figured I was supposed to do that, but it apparently created superfluous changes in the PR having to do with me running the command from a fork. I don't know how to weed those out from the PR at this point, so if you have any suggestions, I'm listening.

Tangent / Future Docs

One strong annoyance that remains is that you can't declaratively specify the document ordering in a single place. I did some research and apparently this is a feature of Jekyll 4, but we seem to be stuck on Jekyll 3 because GitHub isn't supporting the github-pages Ruby gem anymore. See: https://github.com/github/pages-gem/issues/651

In Jekyll 3, reordering doc elements is really spaghetti, because you have to touch N files after you move something.

So one recommendation might be to drop this plugin and move to a different solution so that you can actually get on the latest code. In the thread it talks about how people just manually copy the files in CI.

Zamiell avatar May 07 '22 21:05 Zamiell

There is probably some good stuff in here, but making so many changes in one PR/commit makes it difficult for a maintainer to review it

nschonni avatar May 08 '22 04:05 nschonni

@Zamiell,

Thank you very much for the extensive effort you have put into this.

I was thinking about moving to Docusaurus. Jekyll was used because at the time it was the easiest to get up and running on GitHub Pages.

It is going to take me a while to look through it.

Jason3S avatar May 08 '22 08:05 Jason3S

Yeah, I love Docusaurus myself! That's what typescrult-eslint recently moved to.

Zamiell avatar May 08 '22 13:05 Zamiell

RE: the gem lockfile

It was corrupt, so I deleted it and regenerated it automatically by just typing "bundler install", so this is why the other platform things were removed or whatever. For now, I'll just remove that file from the PR. If you move to Docusaurus, then the corruption won't be an issue, since that doesn't use ruby at all.

Zamiell avatar May 09 '22 10:05 Zamiell

It wasn't corrupt. Bundler just barfs if its version doesn't match the version stored in the lock file. Which is another reason I would prefer to move to Docusaurus, that way everything is JavaScript and not a mix of technologies.

Jason3S avatar May 09 '22 10:05 Jason3S

It wasn't corrupt. Bundler just barfs if its version doesn't match the version stored in the lock file.

Well, what happened to me specifically is that I tried to update the github-pages dep, and it tried to update for hours and never completed, and the StackOverflow suggestion was to just delete the lockfile, which I ended up trying, and it worked.

Which is another reason I would prefer to move to Docusaurus, that way everything is JavaScript and not a mix of technologies.

Agreed! It was a pain in the ass to install Ruby and get this actually working. =p

Zamiell avatar May 09 '22 11:05 Zamiell

I don't think we need this comment. We can filter out zero-width spaces in the documentation.

Ok: https://github.com/streetsidesoftware/cspell/pull/2800/commits/ad84b6ff8ff2c065cf3886bdf38c35bf59efae60

Zamiell avatar May 09 '22 11:05 Zamiell

Additional questions after my latest commit:

  1. What is the difference between the "language", "locale", and "languageId" fields? The JSDoc comments don't say, so this is a bug in the docs.
  2. Where is the list of all valid languages for the "languages" configuration field?
  3. Where is the list of all valid languages for the "locale" configuration field?
  4. Should the docs recommend the use of "language" or "locale" or both?

Zamiell avatar May 10 '22 13:05 Zamiell

@Zamiell,

I appreciate the effort here. The challenge is that there are too many changes in this PR. It makes it very difficult to ensure they are right.

Please make a new PR that includes just your proposed changes to packages/cspell-types/src/CSpellSettingsDef.ts. I think that is a good place to start.

No need to run build-docs, that is done after the PR is merged, otherwise the commit-hash is wrong in all the links.

Based upon your feedback, it is clear some cleaning up is necessary. The key here is to do it in manageable pieces.

Jason3S avatar May 10 '22 18:05 Jason3S

Additional questions after my latest commit:

  1. What is the difference between the "language", "locale", and "languageId" fields? The JSDoc comments don't say, so this is a bug in the docs.
  • language and locale are related to natural language dictionaries.
  • languageId is related to the "programming" language / file type.
  1. Where is the list of all valid languages for the "languages" configuration field?

It is based upon which dictionaries you import. By default, en, en-US, and en-GB included with the spell checker.

  1. Where is the list of all valid languages for the "locale" configuration field?

locale is used to match languages.

  1. Should the docs recommend the use of "language" or "locale" or both?

At the moment, they serve different purposes in the configuration..

Wherever you see language it means "Use this language". language: en, fr - means use English and French.

languageSettings[].locale acts like a filter. If locale intersects language, then apply the settings.

Note: They don't even need to be "valid" natural languages. See: lorem-ipsum/cspell-ext.json

cspell.yaml

overrides:
  - filename: **/test/**
    language: en, lorem

Effectively tells the spell checker to enable the lorem-ipsum dictionary when spell checking test files.

Jason3S avatar May 10 '22 19:05 Jason3S

ok, i rebased this PR on the new main, and fixed the TODO

Zamiell avatar May 11 '22 16:05 Zamiell

@Zamiell,

I have added Docusaurus under cspell/website. It is mostly a default setup with a few things adjusted to get it to work. It is available at cspell.org/docsV2.

I would rather start on migration than on making large changes to the existing docs.

Would you be willing to migrate your change there?

Kind regards, Jason

Jason3S avatar May 15 '22 06:05 Jason3S

Would you be willing to migrate your change there?

Not really, I did the PR before you ever migrated so Docusaurus, so I don't think that's very fair to me.

Zamiell avatar Jun 24 '22 16:06 Zamiell

Jason, any updates on this? The CSpell settings are still not documented.

Zamiell avatar Dec 28 '22 20:12 Zamiell