Skip to content

Refactor character validation to use char.IsAsciiLetter#26270

Closed
xtqqczze wants to merge 1 commit into
PowerShell:masterfrom
xtqqczze:char-IsAscii
Closed

Refactor character validation to use char.IsAsciiLetter#26270
xtqqczze wants to merge 1 commit into
PowerShell:masterfrom
xtqqczze:char-IsAscii

Conversation

@xtqqczze
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov left a 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 })
Copy link
Copy Markdown
Collaborator

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;
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Oct 22, 2025

Choose a reason for hiding this comment

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

Please don't change non-compiled code.

}

return isSupportedDriveForPersistence;
return drive is { Name.Length: 1 } && char.IsAsciiLetter(drive.Name[0]);
Copy link
Copy Markdown
Collaborator

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]));
Copy link
Copy Markdown
Collaborator

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.

@microsoft-github-policy-service microsoft-github-policy-service Bot added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 25, 2025
@microsoft-github-policy-service microsoft-github-policy-service Bot added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept Stale labels Nov 14, 2025
@microsoft-github-policy-service
Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

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

Labels

Stale Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants