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.

Sign up for GitHub

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

Jump to bottom

unix.rs: Added POSIX modifiers support for BCP 47 conversion #33

Merged
merged 1 commit into from
Sep 29, 2024
Merged

unix.rs: Added POSIX modifiers support for BCP 47 conversion #33

merged 1 commit into from
Sep 29, 2024

Conversation

pasabanov
Copy link
Contributor

@pasabanov pasabanov commented Sep 22, 2024 •
edited
Loading

Partially resolves #32.

  1. Renamed function parse_locale_code to posix_to_bcp47.
  2. Changed return type from Option<String> to String.
  3. Changed locale cropping to trim at the first occurrence of either dot (.) or at sign (@), removing any modifiers.
  4. Added tests for modifier cropping.
  5. Added doc comment for posix_to_bcp47.

skibon02
skibon02 reviewed Sep 25, 2024
View reviewed changes
src/unix.rs Show resolved Hide resolved
pasabanov
pasabanov commented Sep 25, 2024
View reviewed changes
src/unix.rs Outdated Show resolved Hide resolved
complexspaces
complexspaces requested changes Sep 26, 2024
View reviewed changes
Copy link
Collaborator

@complexspaces complexspaces left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. I think this is going in the right direction so far.

TIL about the @ separator as well 😄

src/unix.rs Outdated Show resolved Hide resolved
src/unix.rs Outdated Show resolved Hide resolved
src/unix.rs Show resolved Hide resolved
src/unix.rs Show resolved Hide resolved
src/unix.rs Outdated Show resolved Hide resolved
@pasabanov pasabanov changed the title Added POSIX modifiers support for BCP47 conversion Added POSIX modifiers support for BCP 47 conversion Sep 26, 2024
@pasabanov pasabanov changed the title Added POSIX modifiers support for BCP 47 conversion unix.rs: Added POSIX modifiers support for BCP 47 conversion Sep 26, 2024
@pasabanov pasabanov force-pushed the posix_to_bcp47 branch 4 times, most recently from b20dd72 to bfc66e4 Compare September 26, 2024 21:42
@pasabanov
unix.rs: Added POSIX modifiers support for BCP47 conversion
3e33169 
1. Renamed function `parse_locale_code` to `posix_to_bcp47`.
2. Changed return type from `Option<String>` to `String`.
3. Changed locale cropping to trim at the first occurrence of either dot (`.`) or at sign (`@`), removing any modifiers.
4. Added tests for modifier cropping.
5. Added doc comment for `posix_to_bcp47`.
complexspaces
complexspaces approved these changes Sep 29, 2024
View reviewed changes
Copy link
Collaborator

@complexspaces complexspaces left a comment

Choose a reason for hiding this comment

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

Thank you again, this looks good now.

@complexspaces complexspaces merged commit 5ad8ae1 into 1Password:main Sep 29, 2024
42 checks passed
@pasabanov pasabanov mentioned this pull request Sep 29, 2024
Open
2 tasks
@pasabanov pasabanov deleted the posix_to_bcp47 branch October 24, 2024 23:33
@complexspaces
Copy link
Collaborator

Released in 0.3.2. Thanks again!

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

@skibon02 skibon02 skibon02 left review comments

@complexspaces complexspaces complexspaces approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Wrong locale evaluation on Unix-like systems without codeset but with modifier
3 participants
@pasabanov @complexspaces @skibon02