-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
base: main
Are you sure you want to change the base?
Document using extra dicts with pre-commit #207
Conversation
Thank you for giving this a try. |
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. |
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. |
Under docs/ I am considering dropping the |
I'm keeping my changes in separate commits to (hopefully) easily revert something that is not wanted. |
8492335
to
bbb0664
Compare
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 <[email protected]>
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 <[email protected]>
Use Markdown nesting syntax rather than indented code block for for the nested code block. Signed-off-by: Daniel F. Dickinson <[email protected]>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Daniel F. Dickinson <[email protected]>
And drop the README- prefix. This looks better and is more easily transferred to the main docs. Signed-off-by: Daniel F. Dickinson <[email protected]>
Signed-off-by: Daniel F. Dickinson <[email protected]>
This is more consistent with the other dictionary examples and how-tos. Signed-off-by: Daniel F. Dickinson <[email protected]>
bbb0664
to
228514b
Compare
Signed-off-by: Daniel F. Dickinson <[email protected]>
That it is, the configuration does not disable any dictionaries that otherwise apply. Signed-off-by: Daniel F. Dickinson <[email protected]>
Signed-off-by: Daniel F. Dickinson <[email protected]>
Signed-off-by: Daniel F. Dickinson <[email protected]>
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 <[email protected]>
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 ---> |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 usingoverrides
.
@@ -0,0 +1,168 @@ | |||
# Use an Extra Dictionary Based on the Folder or Filename |
There was a problem hiding this comment.
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:
-
Show the problem you are trying to solve
-
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"] }
- In Document:
-
Pick one and explain your choice.
-
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 |
There was a problem hiding this comment.
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` |
There was a problem hiding this comment.
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.
### `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" | ||
|
||
``` |
There was a problem hiding this comment.
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`: |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
I left a lot of comments. I hope you don't find it too daunting. Kind regards, |
@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). |
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. |
@Jason3S these docs have already helped me a lot, how about merging them as is and addressing your remaining comments later? |
@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. |
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. |
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!