-
Notifications
You must be signed in to change notification settings - Fork 238
Fix for #1451 - unicode in mixer should wrap on character boundaries (take 2) #1994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Martin Kaistra <admin@djfun.de>
Signed-off-by: Martin Kaistra <admin@djfun.de>
58bbea9 to
a8853b6
Compare
softins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to fix the issue on my Pi.
|
@jamulussoftware/maindevelopers I can't approve this as I raised the PR. |
|
Currently don’t have time testing/helping out much, but happy to approve. (my studies are challenging – especially dynamic programming is hard 😀). Would it be worth to elect another main-dev (Ignotus,DonC,corrados (?),…)? |
ann0see
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untested
Or maybe dtinth? |
|
Depends on who wants/finds the time and fits the team. I‘d like to see corrados back 😀 but definitely dtinth is another good idea. So the new person would do
Maybe we should move this to a discussion |
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware. The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69. This commit re-applies the fix to new new place (CClientSettingsDlg::OnAliasTextChanged instead of the no longer existing CMusProfDlg::OnAliasTextChanged). It also moves the related inline function to the header to make it possible to include in the first place. Fixes jamulussoftware#1994 (for real). Fixes jamulussoftware#2274. Co-authored-by: Martin Kaistra <admin@djfun.de>
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware. The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69. This commit re-applies the fix to the new place (CClientSettingsDlg::OnAliasTextChanged instead of the no longer existing CMusProfDlg::OnAliasTextChanged). It also moves the related inline function to the header to make it possible to include in the first place. Fixes jamulussoftware#1994 (for real). Fixes jamulussoftware#2274. Co-authored-by: Martin Kaistra <admin@djfun.de>
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware. The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69. This commit re-applies the fix to the new place (CClientSettingsDlg::OnAliasTextChanged instead of the no longer existing CMusProfDlg::OnAliasTextChanged). It also moves the related inline function to the header to make it possible to include in the first place. Fixes jamulussoftware#1994 (for real). Fixes jamulussoftware#2274. Co-authored-by: Martin Kaistra <admin@djfun.de>
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware. The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69. This commit re-applies the fix to the new place (CClientSettingsDlg::OnAliasTextChanged instead of the no longer existing CMusProfDlg::OnAliasTextChanged). It also moves the related function to the header to make inclusion by other files possible in the first place. This also required dropping the `static` and `inline` keywords. Fixes jamulussoftware#1994 (for real). Fixes jamulussoftware#2274. Co-authored-by: Martin Kaistra <admin@djfun.de>
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware. The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69. This commit re-applies the fix to the new place (CClientSettingsDlg::OnAliasTextChanged instead of the no longer existing CMusProfDlg::OnAliasTextChanged). It also adds a function declaration to the header to make inclusion by other files possible in the first place. This also required dropping the `static` and `inline` keywords. Fixes jamulussoftware#1994 (for real). Fixes jamulussoftware#2274. Co-authored-by: Martin Kaistra <admin@djfun.de>
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware. The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69. This commit re-applies the fix to the new place (CClientSettingsDlg::OnAliasTextChanged instead of the no longer existing CMusProfDlg::OnAliasTextChanged). It also adds a function declaration to the header to make inclusion by other files possible in the first place. This also required dropping the `static` and `inline` keywords. Fixes jamulussoftware#1994 (for real). Fixes jamulussoftware#2274. Co-authored-by: Martin Kaistra <admin@djfun.de>
Short description of changes
@djfun wrote a fix but didn't raise a PR, so here's the PR.
Context: Fixes an issue?
Yes. #1451. This still happens.
Does this change need documentation? What needs to be documented and how?
No.
Status of this Pull Request
Needs testing.
What is missing until this pull request can be merged?
Needs testing.
Checklist
I've now created a branch off jamulussoftware/jamulus master and merged @djfun's changes to that, then fixed the conflicts.
I'll try it out now.