-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Refactor character validation to use char.IsAsciiLetter #26270
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
base: master
Are you sure you want to change the base?
Conversation
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.
Please use one PR per change.
| // The drive pointing to VHD in session 'B' gets detected as DriveType.NoRootDirectory | ||
| // after the VHD is removed in session 'A'. | ||
| if (drive != null && !string.IsNullOrEmpty(drive.Name) && drive.Name.Length == 1) | ||
| if (drive is { Name.Length: 1 }) |
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'd prefer to avoid using patterns since we haven't nullability check enabled.
I guess something classic like drive?.Name?.Length == 1 would work here.
| if (char.IsAsciiLetterUpper((char)c)) | ||
| { | ||
| c = (char) (c | 0x20); | ||
| c |= 0x20; |
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.
Please don't check non-compiled code.
| } | ||
|
|
||
| return isSupportedDriveForPersistence; | ||
| return drive is { Name.Length: 1 } && char.IsAsciiLetter(drive.Name[0]); |
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.
The same about pattern.
| // To match old behavior we'll check the drive character for validity as the path is technically | ||
| // not qualified if you don't have a valid drive. "=:\" is the "=" file's default data stream. | ||
| && IsValidDriveChar(path[0])); | ||
| && char.IsAsciiLetter(path[0])); |
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.
Please keep old method name for readability but update implementation.
No description provided.