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

Upgrade to check-spelling v0.0.24 #18261

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

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Nov 28, 2024

Summary of the Pull Request

This upgrades to v0.0.24.

A number of GitHub APIs are being turned off shortly, so you need to upgrade or various uncertain outcomes will occur.

There are some minor bugs that I'm aware of and which I've fixed since this release (including a couple I discovered while preparing this PR).

References and Relevant Issues

Detailed Description of the Pull Request / Additional comments

There's a new accessibility forbidden pattern:

Should be cannot (or can't)

See https://www.grammarly.com/blog/cannot-or-can-not/

Don't use can not when you mean cannot. The only time you're likely to see can not written as separate words is when the word can happens to precede some other phrase that happens to start with not.
Can't is a contraction of cannot, and it's best suited for informal writing.
In formal writing and where contractions are frowned upon, use cannot.
It is possible to write can not, but you generally find it only as part of some other construction, such as not only . . . but also.

  • if you encounter such a case, add a pattern for that case to patterns.txt.
\b[Cc]an not\b

It's not enabled because I'm waiting for:

Validation Steps Performed

https://github.com/check-spelling-sandbox/terminal/actions/
https://github.com/check-spelling-sandbox/terminal/actions/runs/12076096237#summary-33676944561 is what you would have seen if this was running w/o the fixes included
https://github.com/check-spelling-sandbox/terminal/actions/runs/12032865816#summary-33545680702 is what I worked from

PR Checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated
    • If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated (if necessary)

@@ -1265,7 +1247,6 @@ openconsoleproxy
openps
openvt
ORIGINALFILENAME
orking
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is fixed by tweaking the compiler pattern

@@ -58,7 +100,7 @@ equals_insensitive_ascii\("\w+", "\w+"
# hit-count: 109 file-count: 62
# Compiler flags (Unix, Java/Scala)
# Use if you have things like `-Pdocker` and want to treat them as `docker`
(?:^|[\t ,>"'`=(])-(?:D(?=[A-Z])|[WX]|f(?=[ms]))(?=[A-Z]{2,}|[A-Z][a-z]|[a-z]{2,})
(?:^|[\t ,>"'`=(])-(?:D(?=[A-Z])|W(?!ork)|X|f(?=[ms]))(?=[A-Z]{2,}|[A-Z][a-z]|[a-z]{2,})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes the orking item above

check_extra_dictionaries: ""
dictionary_source_prefixes: >
{
"cspell": "https://raw.githubusercontent.com/check-spelling/cspell-dicts/v20241114/dictionaries/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm in the process of updating the cspell dictionaries, so this pulls in what I expect to be the next version with their refreshed content and my refreshed layout. I expect it'll be included in v0.0.25.

- More information on bot-logic that can be controlled with comments is [here](https://github.com/OfficeDev/office-ui-fabric-react/wiki/Advanced-auto-merge)
- See more [information on bot-logic that can be controlled with comments](https://github.com/OfficeDev/office-ui-fabric-react/wiki/Advanced-auto-merge)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one of the cases where the accessibility pattern complained (it's important for people using screen readers)

@@ -25,12 +25,12 @@ Two new properties should be added in the json settings file:

1. All spaces will be ignored.

2. Both X value and Y values are optional. If anyone of them is missing, or the value is invalid, system default value will be used. Examples:
2. Both X value and Y values are optional. If any one of them is missing, or the value is invalid, system default value will be used. Examples:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a manual "in the neighborhood" change.

- It would need to track a the MRU for windows, so pressing the shortcut when
- It would need to track the MRU for windows, so pressing the shortcut when
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is another forbidden pattern (two articles in a row). Corrections to this require a lot more brain power than I've had of late. I could easily be wrong for each instance...

2. Search is performed on a XAML TextBox. Once the user presses Enter or click up/down arrow button, the search starts and searched text will be selected. Next search will be performed beginning from the current selection and go towards up/down.
3. The user can choose to search up or down by selecting up arrow or down arrow buttons. The chosen button will be styled to indicate it is selected. If the user does not click the arrows buttons, the default direction is up.
4. The user can choose to do case-sensitive or insensitive match by checking a check box. The default is case-insensitive.
5. If the search box is focused, the user can navigate all the elements on the search box using tab. When selected, pressing Enter is equivalent to to click.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is trying to address that equals to. Here the fix was more complicated than an obvious search+replace.

#include <Unknwn.h>
#include <unknwn.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the repository uses the lowercase name. I'm not quite sure what the story is for this file, but on most Windows computers the file system is case-preserving and case-insensitive so it doesn't matter. It is possible to mount NTFS as case-sensitive in which case a mismatch would cause a failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should note that while this repository mostly spells this file name in lowercase, most instances of this file reference in microsoft/PowerToys has it with the initial U... I don't have an SDK handy, so I'm not sure which is canonical, but if someone could check, I'd appreciate it.

@@ -244,7 +244,7 @@ std::unordered_map<std::wstring,
exeNameW));

// Set the return size copied to the size given before we attempt to copy.
// Then multiply by sizeof(wchar_t) due to a long standing bug that we must preserve for compatibility.
// Then multiply by sizeof(wchar_t) due to a long-standing bug that we must preserve for compatibility.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CJR has an article and notes that longstanding has not yet won: https://www.cjr.org/language_corner/long-hyphen.php
But, in general, long standing isn't really preferred by anyone afaict...
https://www.merriam-webster.com/dictionary/long-standing

@@ -1816,14 +1792,14 @@ texttests
TFunction
THUMBPOSITION
THUMBTRACK
tickit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This appears because of a new pattern that strips off lib prefixes

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.

1 participant