<feat>: use short_description #18
Conversation
|
Thanks! Do take a minute to make sure that the linters pass. |
src/gui/usbipd_gui.rs
Outdated
| let menu_item = self | ||
| let menu_item; | ||
|
|
||
| if description.len() > 16 { |
There was a problem hiding this comment.
It seems that this should be more like ~32. As is, it has the potential to make the name longer while conveying less information.
There was a problem hiding this comment.
Agreed on increasing the length to 32, I feel like 16 would be too short to convey any useful information.
There was a problem hiding this comment.
A better way maybe use menu_item to config.
if agree with it, i will cost some time to achieve it .( i have no exp with nwg, so i need read code to understand)
There was a problem hiding this comment.
Why not just increase the number to 32? Is it more complicated than that?
src/gui/usbipd_gui.rs
Outdated
|
|
||
| if description.len() > 16 { | ||
| let mut short_description = description.clone(); | ||
| short_description.truncate(16); |
There was a problem hiding this comment.
You can avoid the if-else here right away and just always call truncate on the string, since truncate does nothing if the string is already shorter, (see rust doc about truncate). This should simplify the code and reduce nesting a bit, making everything cleaner.
There was a problem hiding this comment.
if len less 16, maybe we needn't show bus id. so i use if-else. now i not have a batter code to active it, does you have some suggestions?
There was a problem hiding this comment.
Oh I see now why you did it like this. Fine by me, keep it as is.
| // if len > 32, truncate and add bus id | ||
| if description.len() > 32 { // @todo use settings replace magic number | ||
| description.truncate(32); | ||
| let bus_id = device | ||
| .bus_id | ||
| .clone() | ||
| .unwrap_or_else(|| "Unknown Bus".to_string()); | ||
| description.push_str(&format!("..(Bus:{})", bus_id)); | ||
| } |
There was a problem hiding this comment.
I think this is not the desired behavior. If the goal is to establish a max string length, truncating if necessary, and adding unique ID (bus ID), then the length of that metadata must be taken into account. The present implementation can result in a longer string with less information.
The truncation point is not the same as the desired max string length. The truncation point must be max_length - length(id_string).
- description is greater than max length
- format the ID string (... BUS-ID)
- truncate and concat at description[max_length - length(id_string)]
This will prevent the fix from being longer than the problem.
nickbeth
left a comment
nickbeth
left a comment
There was a problem hiding this comment.
LGTM, I'll briefly test it and merge if all goes smoothly. Thanks!
3 participants
if device info Description have a long info, the tray menu will very long.
so we need limit description.
besides, we need show bus-id, if user use same device
more infore
#13