-
Notifications
You must be signed in to change notification settings - Fork 176
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
Split unfold functionality into separate type #3759
Conversation
a732f67
to
44c10dd
Compare
44c10dd
to
8c08cec
Compare
/// In other words, this adds all strings/characters that this casemaps to, as | ||
/// well as all characters that may casemap to this one. |
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.
Question: Does this include all case mapping operations, or only case folding?
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.
all mapping
experimental/casemap/src/closer.rs
Outdated
self.cm.as_ref().add_case_closure(c, set); | ||
} | ||
|
||
/// Finds all characters and strings which may casemap to `s` and adds them to the set. |
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.
Question: does it also include all characters and strings which s
casemaps to? Or only those which casemap to s
?
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.
No, just the reverse
experimental/casemap/src/closer.rs
Outdated
/// let cm = CaseMapCloser::new(); | ||
/// let mut builder = CodePointInversionListBuilder::new(); | ||
/// let found = cm.add_string_case_closure("ffi", &mut builder); | ||
/// assert!(found); | ||
/// let set = builder.build(); |
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.
Question: what happens if my string is ffiss
? Do I get ffiss
, ffiß
, and ffiß
all added to my set?
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.
nothing, it's only for the atomic casemappings.
I don't think there's good terminology around this unfortunately.
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.
LGTM modulo _to
Part of #3234
(the first commit is a related minor change that I'm sneaking in)
Some things I'm not sure about:
&CaseMapper
as an argumentWhatever design we end up with here will probably be exactly what I do for titlecase as well!