-
Notifications
You must be signed in to change notification settings - Fork 189
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
fix: ToLower/ToUpper to ToLowerInvariant/ToUpperInvariant #921
Conversation
Added extra spaces/tabs to not have diff |
@@ -27,7 +27,7 @@ public static class ScreenOrientationExtensions | |||
/// <param name="orientation"></param> | |||
/// <returns></returns> | |||
public static string JSONWireProtocolString(this ScreenOrientation orientation) => | |||
orientation.ToString().ToUpper(); | |||
orientation.ToString().ToUpperInvariant(); |
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.
Related tests passed?
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 tested the enum to string conversion on my local with ToUpperInvariant
, while my env didn't cause the #920, so it tested the output didn't change with ToUpper
though
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.
Pull Request Overview
This PR fixes locale-dependent string casing in various parts of the .NET client by replacing ToLower/ToUpper with their invariant counterparts.
- Replaces ToLower with ToLowerInvariant in capability parsing and environment evaluation
- Updates screen recording and orientation options to use invariant casing conversions
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/Appium.Net/Appium/Service/Options/OptionCollector.cs | Updated boolean value conversion for capabilities using ToLowerInvariant |
test/integration/helpers/Env.cs | Adjusted environment variable conversion to use ToLowerInvariant |
src/Appium.Net/Appium/iOS/IOSStartScreenRecordingOptions.cs | Changed video type/quality conversion to ToLowerInvariant |
src/Appium.Net/Appium/ScreenOrientationExtensions.cs | Replaced ToUpper with ToUpperInvariant for consistent casing |
List of changes
I've learned in #920, we should use
ToLowerInvariant
/ToUpperInvariant
to avoid host machine language config in the string conversion. This PR applies the same change what #920 does for clipboard for others.Types of changes
What types of changes are you proposing/introducing to the .NET client?
Put an
x
in the boxes that applyDocumentation
This can be done by navigating to the documentation section on http://appium.io selecting the appropriate command/endpoint and clicking the 'Edit this doc' link to update the C# example
Integration tests
Details
Please provide more details about changes if necessary. You can provide code samples showing how they work and possible use cases if there are new features. Also, you can create gists with pasted C# code samples or put them here using markdown.
About markdown please read Mastering markdown and Writing on GitHub