Disabling Isolation Marks in Fluent #46
Conversation
|
I repinned my |
|
Here's the usage in a commit which outputs the text as expected: fosskers/aura@53c657d#diff-c81b5030f3913711f46fce5ce5bc3cce4be2e370de609b1830661df8474fce11R18 |
| /// | ||
| /// Default: `true`. | ||
| pub fn set_use_isolating(&self, value: bool) { | ||
| for bundle in self.language_config.write().language_bundles.as_mut_slice() { |
There was a problem hiding this comment.
Rust surprised me here. I'm able to get mutable access to the a nested field several levels down, and yet we're borrowing the parent immutably with &self.
And a question with regard to write(): I see from its docs that it blocks the thread. Is my overall approach to getting down to the internal bundle a good one?
There was a problem hiding this comment.
language_config is wrapped in an RwLock which gives it safe interior mutability with the use of runtime checks. The downside in this case is that threads will block if they try to access it when someone holds the write lock to it, but I hoped that it would not be common to be making changes to the language_config during a performance critical piece of code, thus it would at least be better than using a Mutex. It also has the property that the write lock cannot be obtained until all the read locks have been dropped.
I guess we need to be careful not to introduce a deadlock if this method will be called within another method that already holds the lock, seems like that's not the case here so it looks good to me.
| /// | ||
| /// Default: `true`. | ||
| pub fn set_use_isolating(&self, value: bool) { | ||
| for bundle in self.language_config.write().language_bundles.as_mut_slice() { |
There was a problem hiding this comment.
language_config is wrapped in an RwLock which gives it safe interior mutability with the use of runtime checks. The downside in this case is that threads will block if they try to access it when someone holds the write lock to it, but I hoped that it would not be common to be making changes to the language_config during a performance critical piece of code, thus it would at least be better than using a Mutex. It also has the property that the write lock cannot be obtained until all the read locks have been dropped.
I guess we need to be careful not to introduce a deadlock if this method will be called within another method that already holds the lock, seems like that's not the case here so it looks good to me.
| /// information. | ||
| /// | ||
| /// Default: `true`. | ||
| pub fn set_use_isolating(&self, value: bool) { |
There was a problem hiding this comment.
perhaps we could add a unit/integration test for this?
There was a problem hiding this comment.
Yes, perhaps it would sufficient to check the byte lengths of the output strings? I'll add a commit today.
There was a problem hiding this comment.
I guess so, or to be more specific you could check if the string contains the expected special characters or not. One test with it enabled, and one without it enabled.
|
Done! Should be good to go. Note the comment I added about calling |
|
Awesome! Looks good now, I'll try to merge it and push a new release today, thanks for all the help! |
This PR is a proposed solution to #45.