Skip to content

Conversation

@pljones
Copy link
Collaborator

@pljones pljones commented Sep 5, 2021

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 verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

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.

djfun added 2 commits April 6, 2021 18:15
Signed-off-by: Martin Kaistra <admin@djfun.de>
Signed-off-by: Martin Kaistra <admin@djfun.de>
@pljones pljones force-pushed the djfun-devel/unicode-name branch from 58bbea9 to a8853b6 Compare September 5, 2021 21:56
@softins softins added this to the Release 3.9.0 milestone Oct 12, 2021
@softins softins linked an issue Oct 25, 2021 that may be closed by this pull request
Copy link
Member

@softins softins left a 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.

@softins softins changed the title Fix for #1688 - unicode in mixer should wrap on character boundaries (take 2) Fix for #1451 - unicode in mixer should wrap on character boundaries (take 2) Oct 25, 2021
@pljones
Copy link
Collaborator Author

pljones commented Nov 13, 2021

@jamulussoftware/maindevelopers I can't approve this as I raised the PR.

@ann0see
Copy link
Member

ann0see commented Nov 13, 2021

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 (?),…)?

Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested

@pljones pljones merged commit 5158996 into master Nov 13, 2021
@pljones pljones deleted the djfun-devel/unicode-name branch November 13, 2021 12:11
@softins
Copy link
Member

softins commented Nov 13, 2021

Would it be worth to elect another main-dev (Ignotus,DonC,corrados (?),…)?

Or maybe dtinth?

@ann0see
Copy link
Member

ann0see commented Nov 14, 2021

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

  • maintenance:
  • testing/reviewing PRs (—> the person should have at least basic coding experience)
  • Get write access to all GitHub repos (—> the person should be trustworthy)
  • moderate the Community and encourage positive behaviour (—> the person should be social)
  • should follow https://opensource.guide/best-practices/

Maybe we should move this to a discussion

@gilgongo gilgongo modified the milestones: Release 3.9.0, 3.8.2 Jan 17, 2022
hoffie added a commit to hoffie/jamulus that referenced this pull request Jan 25, 2022
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>
hoffie added a commit to hoffie/jamulus that referenced this pull request Jan 25, 2022
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>
hoffie added a commit to hoffie/jamulus that referenced this pull request Jan 25, 2022
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>
hoffie added a commit to hoffie/jamulus that referenced this pull request Jan 25, 2022
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>
hoffie added a commit to hoffie/jamulus that referenced this pull request Jan 25, 2022
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>
hoffie added a commit to hoffie/jamulus that referenced this pull request Jan 25, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Musician name wrapping cuts through Unicode Symbols

6 participants