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

Document using extra dicts with pre-commit #207

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

danielfdickinson
Copy link

In particular, the overrides doc might better be part of the main docs on the website, but I admit to being confused about where and how to contribute to the main docs at the present time.

I'd love to help with them though.

In the meantime I hope you find this documentation effort useful.

I'm happy to make any changes you request. Thank you for great software!

docs/README-OVERRIDES.md Outdated Show resolved Hide resolved
docs/README-OVERRIDES.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/README-OVERRIDES.md Outdated Show resolved Hide resolved
docs/README-OVERRIDES.md Outdated Show resolved Hide resolved
@Jason3S
Copy link
Contributor

Jason3S commented Aug 19, 2022

@danielfdickinson,

Thank you for giving this a try.

docs/README-OVERRIDES.md Outdated Show resolved Hide resolved
@danielfdickinson
Copy link
Author

danielfdickinson commented Aug 19, 2022

Thank you for the review, will update accordingly, soon. I appreciate the suggestions, I think they make sense. I am happy to have to chance to have a second opinion on what I write; that hasn't been the case for a while, to my documentation's detriment.

@danielfdickinson danielfdickinson marked this pull request as draft August 19, 2022 23:39
@danielfdickinson
Copy link
Author

I've converted this PR to a draft temporarily as I am making substantial changes based on the review comments (as you will see shortly), and want you to see things as they develop rather than waiting until I've made all my changes.

@danielfdickinson
Copy link
Author

Under docs/ I am considering dropping the README- prefix and lower casing the names. Does that make sense to you?

@danielfdickinson
Copy link
Author

I'm keeping my changes in separate commits to (hopefully) easily revert something that is not wanted.

@danielfdickinson danielfdickinson force-pushed the document-using-extra-dicts-with-pre-commit branch from 8492335 to bbb0664 Compare August 20, 2022 02:27
Document using included dictionaries on files for which they are not normally used
(for example, Bash words on Markdown or text files), when using `pre-commit`.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
This closes streetsidesoftware#206 and stems from
streetsidesoftware/cspell#3426
where I had to dig around rather a lot to pull the pieces together.
Hopefully the docs make life easier for the next person who wants
to do this kind of thing.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Use Markdown nesting syntax rather than indented code block for
for the nested code block.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Prevent GitHub workflow (CI) from failing on run of CSpell over the examples
of words not in the default dictionaries, when the workflow is using only
the default dictionaries.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Per review comments, tidy up the README references to the how-tos and
configuration examples, and use a title / format to which more docs can
be easily added.

In the process, add a preliminary version of an example setup for French.
Currently a copy of the extra-dics example/how-to but will become a
unique document.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
And drop the README- prefix. This looks better and is more easily transferred
to the main docs.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
This is more consistent with the other dictionary examples and how-tos.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
@danielfdickinson danielfdickinson force-pushed the document-using-extra-dicts-with-pre-commit branch from bbb0664 to 228514b Compare August 20, 2022 02:43
Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
That it is, the configuration does not disable any dictionaries
that otherwise apply.

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
As noted, this mostly (only?) applies to language dictionaries as other
dictionaries are generally included by default, but only applied to
certain contexts (often a VSCode languageId for the programming language
dictionaries).

Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>
@danielfdickinson danielfdickinson marked this pull request as ready for review August 20, 2022 04:33
@danielfdickinson
Copy link
Author

I'm ready for the next round of comments.

@@ -0,0 +1,168 @@
# Use an Extra Dictionary Based on the Folder or Filename
<!--- cspell:ignore esac getopts shopt --->
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: it is also possible to enable a dictionary in a file if adding words becomes too long.

<!--- cspell:dictionaries bash --->

# Use an Extra Dictionary Based on the Folder or Filename
<!--- cspell:ignore esac getopts shopt --->

For example, using Bash dictionary with Markdown or text files, by filename (the
Copy link
Contributor

Choose a reason for hiding this comment

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

How about flipping the logic:

By default the Bash dictionary is only applied to files of type schellscript. In this example we will show how to enable the Bash dictionary based upon a file glob using overrides.

@@ -0,0 +1,168 @@
# Use an Extra Dictionary Based on the Folder or Filename
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there are a few approaches to document this. A couple come to mind:

  • A walk through style.
  • A reference style.

I think either approach is fine.

Based upon how you wrote it so far, I'm guessing you are leaning towards a Walk Through Style.
My suggestion is to use a single short example file. No need to show too much. We just want
to demonstrate the issue to be solved.

Walk Though Style

A walk through is more like telling a story:

  1. Show the problem you are trying to solve

    • lots of spelling issues in bash code blocks.
    • Screen shots:
      image or image
  2. Show some ways to solve it.

    • In Document: <!--- cspell:ignore esac ---> or <!--- cspell:words esac --->
    • In Document: <!--- cspell:dictionaries bash --->
    • Using overrides
    • Using languageSettings
    • Add words to cspell.json: { "words": ["esac"] }
  3. Pick one and explain your choice.

  4. Summarize / conclusions.

Useful things:

  • Some screen shot could be useful.
  • Keep configuration short.
  • The should be a logical flow.

Reference Style

In a reference style, it is more like a list of examples.

  • Enable the bash dictionary using overrides
  • Enable the bash dictionary using languageSettings
  • Enable the bash dictionary using in-document settings.

Tools

  • list of dictionaries:
    cspell-cli trace word
  • cli based visual check:
    cspell-cli check docs/file-or-folder-based-overrides.md

**Note**: This adds the Bash dictionary to the list of dictionaries against
which to check the files, it does not check only against the Bash dictionary.

This is the same for use with `pre-commit` as with other methods of invoking
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, pre-commit works exactly the same. I think it is fine to mention that in the summary.

- id: cspell
```

#### Triggering CSpell using `pre-commit`
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move the pre-commit directions into it own file: Using cspell-cli with pre-comit.

Comment on lines +6 to +22
### `pre-commit-config.yaml` configuration

Extend the `pre-commit` hook config from the [README.md](../README.md) with
`additional_dependencies`. For example:

```yaml
# .pre-commit-config.yaml
repos:
- repo: https://github.com/streetsidesoftware/cspell-cli
rev: v6.7.0
hooks:
- id: cspell
additional_dependencies:
- "@cspell/dict-fr-fr"
- "@cspell/dict-fr-reforme"

```
Copy link
Contributor

Choose a reason for hiding this comment

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

Great!

Voici, nous avons les mots française
```

as again as `mots-française.err`:
Copy link
Contributor

@Jason3S Jason3S Aug 20, 2022

Choose a reason for hiding this comment

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

I don't think you need to repeat the example or show errors.

At most, a directory tree showing which files will use French and which ones will not.

When `cspell` is invoked `mots-française.md` should not show errors, but
`mots-française.err` should.

## Invoking CSpell
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a list of references to other pages.

@Jason3S
Copy link
Contributor

Jason3S commented Aug 20, 2022

@danielfdickinson,

I left a lot of comments. I hope you don't find it too daunting.

Kind regards,
Jason

@danielfdickinson
Copy link
Author

@Jason3S I appreciate the comments quite a lot. I think it is great for getting back into 'real work' mode (I've been off work for a few years due to a major crisis, and your detailed explanations and suggestions are really reminding me how get to the level I want to be at).

@danielfdickinson
Copy link
Author

Sorry for the delay on this; I haven't forgotten, just haven't had the right combination of available time and energy to work on this.

@flying-sheep
Copy link

@Jason3S these docs have already helped me a lot, how about merging them as is and addressing your remaining comments later?

@danielfdickinson
Copy link
Author

danielfdickinson commented Nov 8, 2022

@flying-sheep Hi! I'm glad the docs have helped as is 🙂

I had quite a bit of 'life stuff' come up, but I also got into what I thought was a shorter term side project that I was focusing on. Since I've realized that it won't be done as quickly as I had expected, I'm planning on scheduling that for a longer term, and making time for this. (Thank you for reminding me of it).

My thought is to make the main doc a reference style. I may also add a small walk-through style, but that remains to be seen.

That said, I wonder if it'd be better to merge as a clearly labelled work in progress, and do pull requests to modify separately rather than to try to have an ideal single pull request. That's up to @Jason3S though.

@flying-sheep
Copy link

Yeah, that’s what I had in mind! Better to get things out there instead of leaving them in a hard to find spot for extended periods of time.

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

Successfully merging this pull request may close these issues.

None yet

3 participants