-
Notifications
You must be signed in to change notification settings - Fork 909
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 versionsort chunk split on non-ASCII numerics #6407
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
use std::cmp::Ordering; | ||
use print๙msg::print as first_print; | ||
use print0msg::print as second_print; | ||
use printémsg::print as third_print; | ||
|
||
fn main() { | ||
first_print(); | ||
second_print(); | ||
third_print(); | ||
|
||
assert_eq!("print๙msg".cmp("printémsg"), Ordering::Greater); | ||
} | ||
|
||
/// '๙' = 0E59;THAI DIGIT NINE;Nd; | ||
mod print๙msg { | ||
pub fn print() { | ||
println!("Non-ASCII Decimal_Number") | ||
} | ||
} | ||
|
||
/// '0' = 0030;DIGIT ZERO;Nd; | ||
mod print0msg { | ||
pub fn print() { | ||
println!("ASCII Decimal_Number") | ||
} | ||
} | ||
|
||
/// 'é' = 00E9;LATIN SMALL LETTER E WITH ACUTE;Ll; | ||
mod printémsg { | ||
pub fn print() { | ||
println!("Lowercase_Letter") | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove any code that isn't an import since it's not needed for the test case. Though, It would be great to keep the explanatory comments you added about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated and expanded on the comments to explain why they sort in that order. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
use print0msg::print as second_print; | ||
use printémsg::print as third_print; | ||
use print๙msg::print as first_print; | ||
use std::cmp::Ordering; | ||
|
||
fn main() { | ||
first_print(); | ||
second_print(); | ||
third_print(); | ||
|
||
assert_eq!("print๙msg".cmp("printémsg"), Ordering::Greater); | ||
} | ||
|
||
/// '๙' = 0E59;THAI DIGIT NINE;Nd; | ||
mod print๙msg { | ||
pub fn print() { | ||
println!("Non-ASCII Decimal_Number") | ||
} | ||
} | ||
|
||
/// '0' = 0030;DIGIT ZERO;Nd; | ||
mod print0msg { | ||
pub fn print() { | ||
println!("ASCII Decimal_Number") | ||
} | ||
} | ||
|
||
/// 'é' = 00E9;LATIN SMALL LETTER E WITH ACUTE;Ll; | ||
mod printémsg { | ||
pub fn print() { | ||
println!("Lowercase_Letter") | ||
} | ||
} |
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.
version sort is only be applied when using
style_edition=2024
. You'll need to configure that for this test as follows:You can read more about the comment configuration in test cases here.
Also, can you also add a
style_edition=2015
import sorting test case so that we can compare the pre version sort ordering.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.
Added. The 2015 edition actually sorts the same way since
U+0030 < U+00E9 < U+0E59
, so the bug only affected 2024 edition (and can't be reproduced on earlier versions because ASCII digits are the earliest Unicode numerics).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.
Thanks for adding that additional test case!