Truncate display names with longer than 30 chars #171
Conversation
micbakos-rdx
requested review from
CyonAlexRDX,
GhenadieVP,
jakub-rdx,
giannis-rdx,
danvleju-rdx,
sergiupuhalschi-rdx and
matiasbzurovski
| found: value.len() as u64, | ||
| }); | ||
| } | ||
| value.truncate(Self::MAX_LEN); |
There was a problem hiding this comment.
use the same you used in recent PR? .chars().take(Self::MAX_LEN).collect() instead? which seemed safe.?
Clever idea, will do that |
src/core/error/common_error.rs
Outdated
| @@ -230,6 +230,7 @@ pub enum CommonError { | |||
| #[error("Invalid DisplayName cannot be empty.")] | |||
| InvalidDisplayNameEmpty = 10062, | |||
|
|
|||
| /// WARNING: UNUSED, can be replaced by any new error. | |||
There was a problem hiding this comment.
No warning needed imo. Just rename the variant to FREE and the error message
|
|
||
| Ok(Self { value }) | ||
| Ok(Self { | ||
| value: value.chars().take(Self::MAX_LEN).collect(), |
There was a problem hiding this comment.
Since we do this in two places, might as well write a func or method for it, put it in string_utils and add tests for it.
You can perhaps extend the trait StrExt
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
=======================================
- Coverage 98.8% 98.8% -0.1%
=======================================
Files 756 756
Lines 11348 11347 -1
Branches 27 27
=======================================
- Hits 11213 11212 -1
Misses 133 133
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
CommonError::InvalidDisplayNameTooLongin now unused. I purposely did not delete it. Is it safe to remove it and fix the error codes of the rest of the following errors?truncate()function. This might panic if "new_len does not lie on a char boundary." Can someone help with suggestions on how this might fail? Or even suggest a better function?