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

docs: Improved spellchecking #2722

Merged
merged 9 commits into from
Sep 11, 2023
Merged

docs: Improved spellchecking #2722

merged 9 commits into from
Sep 11, 2023

Conversation

luanpotter
Copy link
Member

@luanpotter luanpotter commented Sep 10, 2023

Description

Improve our spellchecker (cspell) configuration and dictionary file organization.

Rationale

This is a proposal to establish a few changes:

  • better separate our dictionary files into different categories of types of words we are including
  • improve the cspell regexes to be more aggressive
  • be less lenient to what kinds of words we are adding to our dictionaries
  • have the dictionary file also serve as an explanation for obscure references that cannot be easily derived from the word

Essentially my goal is that either when reviewing a PR that adds a new entry to our dictionary, or when reading the dictionary files themselves, it is immediately obvious what the entries are and why they are there. Currently it can be just a dumpster we throw anything into if spellcheck fails.

Proposal

This PR-as-a-proposal essentially do the following changes.

Split Dictionary Files

Proposes a better separation for our dictionary files. Currently we have 3 that are a bit broad and not super clear on what goes where. This breaks it down a bit more and adds a comment to each file explaining what kinds of terms should be added to each; that also serves as a general guidance for what kinds of words should be added to the lexicon in general, and makes it harder for mistakes to make into it.

  • flame_dictionary: remains pretty much unchanged; it is dedicated to Flame-related words, including companies, tools, and libraries (and their associated concepts) mentioned on our codebase. Basically a collection of proper-nouns relating to companies and libraries we mention.
  • dart_dictionary: new file for Dart and Flutter related terms
  • sphinx_dictionary: unchanged, for Sphynx related terms
  • people_dictionary: specific for people names and usernames referenced on the codebase (in TODOs, mentions, etc)
  • words_dictionary: actual English-language words (or common abbreviations) missing from CSpell
  • gamedev_dictionary: this was our biggest file that contained all sort of things. it has been mostly broken down and now only contains general development-adjacent terms and expressions

Include definitions

Except for the words dictionary, which should be self-explanatory (as it basically covers for "holes" in CSpell standard dictionary, which I have been finding a bit lacking), every other file will contain terms in the form:

word # definition of the word

What exactly the definition is can slightly vary depending on which dictionary file we are talking about, but the examples should be self-explanatory.

As an example, for the gamedev file, it should provide some simple guidance as to what the term means, or if it's an acronym or abbreviation, what it stands for. The goal is not to teach the entire concept to someone unfamiliar, but allow them to "google" it for themselves by giving enough context, so they can confirm their suspicions. For example, if they see LTRB somewhere by itself, they are not able to "just google that" because it is too vague. The dictionary file provides enough context for the user to figure out however much deeper information they want about any particular subject. It will also disambiguate from any non-Flame related homonyms. For people on the people file and companies on the flame file, the description will provide links to clearly disambiguate what they are; for tools, a brief description of what the tool is for is also included. And so on.

The goal is not to build a comprehensive, in depth-guide to each word we use, but rather to give the bare minimum of context on what this term "is doing" on our codebase.

Be less lenient with terms

My idea with these two major changes combined, is that we are overall more tactical about which terms we want to add to the dictionaries. Adding a word to the dictionary file is essentially giving carte blanche to anyone in the future to reuse that term anywhere. I think we should see spellchecker violations as "warnings"; we decide on the set of warning rules we want to enable for the entire project (hopefully all the ones that make sense; or have a reason for disabling the ones that don't). We might need to violate these warnings sporadically, for example, we ban print on the codebase but might need to allow it specifically in a couple places. But we would not disable the entire warning to do that, rather we would add a specific comment-bypass on the smallest possible scope that encompasses all the relevant lines. We would also add a proper comment explaining why we are bypassing the general rule in this specific place.

Similarly, we should not have one-off violations on the dictionary file, even if they make sense in the one place they occur, but we should encourage more liberal use of scoped bypasses for such cases. These Ukrainian words are required in this file, but should not be on the dictionary as it does not make sense to use foreign languages anywhere else:

// used as examples of Ukrainian words on the documentation below
// cSpell:ignore рушниця, рушниці, рушниць

It might look inelegant to have to include that, but just like a warning-bypass comment, accompanied by the explanatory proper-comment, this actually provides helpful guidance and context for the reader that might be confused with the usage of incomprehensible terms.

This also encourages people to avoid obscure terms that are not already in our dictionary (i.e. that we have already "bought in" and paid the mental load investment cost), making our code (and docs) easier to parse and read for everyone. I want to be extremely clear that that does not mean we need to "dumb down" anything whatsoever, or do any sort of gymnastics to avoid the wrath of an incompetent spellchecker.

But, for example when spelling "cave ace" in variable names in a random example, having it typed as caveAce instead of caveace can slightly help with readability, specially for non-native speakers (like most of us). It is an extremely minor insignificant gain, but having the dictionary file require a brief description will nudge us to give a bit more thought into each "bypass" we are adding.

(note: a similar issue that I have not yet addressed is "spine boy", but I will leave that for followups and just added that one to the dictionary for now, as I am still over the fence on that one since it is an actual "known" character with a dedicated page, so it is more like a proper noun - as a specific decision I think it is out-of-scope of the broader discussion).

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

@@ -14,7 +14,7 @@ import 'package:flame/math.dart';
/// components.
/// If you want to use a non static time interval, use the
/// [SpawnComponent.periodRange] constructor.
/// {@endremplate}
/// {@endtemplate}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note how a more aggressive regex can catch actual bugs and save lives! (well at least the former)

@luanpotter luanpotter changed the title docs: Improved spellchecking [DRAFT - WIP - DO NOT REVIEW] docs: Improved spellchecking [Proposal] Sep 10, 2023
@luanpotter luanpotter marked this pull request as ready for review September 10, 2023 23:25
@luanpotter luanpotter changed the title docs: Improved spellchecking [Proposal] docs: Improved spellchecking Sep 11, 2023
@luanpotter luanpotter merged commit 2f973ab into main Sep 11, 2023
7 checks passed
@luanpotter luanpotter deleted the luan.cspell branch September 11, 2023 16:24
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

Successfully merging this pull request may close these issues.

2 participants