Unicode collation #551
Unicode collation #551
Conversation
|
Before merge, we'd also prefer a cleaner git history (I'm a little nitpicky here :)). Depending on your git expertise, we may help you with that.
|
|
Checklist done. |
d-sonuga
requested review from
phip1611 and
nicholasbishop
and removed request for
phip1611 and
nicholasbishop
| ); | ||
| } | ||
| } else { | ||
| warn!("The Unicode Collation protocol is not supported"); |
There was a problem hiding this comment.
this should panic. Or do the other tests do it the same way?
I think if the OVMF version of our tests suddenly doesn't include a protocol anymore, we want to get notified. What does @nicholasbishop think?
There was a problem hiding this comment.
A number of the other tests do it the same way.
There was a problem hiding this comment.
I think it's fine for now, as you say a lot (but not all) of the tests have such a check.
I filed #553 for us to figure this out properly, but that can happen outside the context of this PR.
| info!("Testing the Unicode Collation protocol"); | ||
|
|
||
| if let Ok(handles) = bt.find_handles::<UnicodeCollation>() { | ||
| for handle in handles { |
There was a problem hiding this comment.
I'm not sure if this makes sense.. but I'm not an expert with the uefi-API. Why would there be multiple handles to the same protocol? @GabrielMajeri @nicholasbishop
There was a problem hiding this comment.
It looks like this protocol can have multiple instances (i.e. multiple handles). From section 21.1.1:
"One or more of the EFI_UNICODE_COLLATION_PROTOCOL instances may be present at one time.
Each protocol instance can support one or more language codes."
|
LGTM, thanks for working on this @d-sonuga ! Only a few more things and then this will be ready for merge! |
|
I think everything's been addressed, lgtm :) |
Added support for the Unicode Collation protocol.
Checklist