Utf8 prompts#281
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #281 +/- ##
==========================================
- Coverage 48.70% 48.33% -0.38%
==========================================
Files 11 11
Lines 1043 1051 +8
==========================================
Hits 508 508
- Misses 535 543 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| macro_rules! set_status { ($editor:expr, $($arg:expr),*) => ($editor.status_msg = Some(StatusMessage::new(format!($($arg),*)))) } | ||
|
|
||
| // `width!` returns the display width of a string, plus one for the cursor | ||
| fn dsp_width(msg: &String) -> usize { UnicodeWidthStr::width(msg.as_str()) + 1 } |
There was a problem hiding this comment.
It looks like this function is only used once, is it could it be inlined, or does that mess up the line count?
| Cancelled, | ||
| } | ||
|
|
||
| thread_local! (static CHARACTER: RefCell<Vec<u8>> = {let cache = Vec::new(); RefCell::new(cache)}); |
There was a problem hiding this comment.
I'd rather avoid thread local static in this code, could this be added as a field to PromptMode::Find etc. instead (similar to the string buffer)?
| let character = CHARACTER.with(|cache| String::from_utf8(cache.borrow_mut().clone())); | ||
| let _ = character.clone().map_or((), |c| buffer.push_str(c.as_str())); | ||
| let _ = character.map_or((), |_| CHARACTER.with(|cache| cache.borrow_mut().clear())); |
There was a problem hiding this comment.
I think this would be clearer as a single if instead of 3 assignments, something like
if let Ok(s) = std::str::from_utf8(&cache) {
buffer.push_str(s);
cache.clear();
}What do you think?
Note that using std::str::from_utf8 instead of String::from_utf8 avoids allocating an extra string, and there is no need for mutable borrow + cloning, a simple borrow is enough
Signed-off-by: Ilaï Deutel <ilai-deutel@users.noreply.github.com>
#5 Issue solved!