Skip to content
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

fix #305134: crash on chordsymbol playback #6071

Merged
merged 2 commits into from
May 12, 2020

Conversation

Jojo-Schmitz
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz commented May 9, 2020

No description provided.

const char* Channel::DEFAULT_NAME = QT_TRANSLATE_NOOP("channel", "Normal");
const char* Channel::HARMONY_NAME = QT_TRANSLATE_NOOP("channel", "Chord symbols");
const char* Channel::DEFAULT_NAME = QT_TRANSLATE_NOOP("InstrumentsXML", "Normal");
const char* Channel::HARMONY_NAME = QT_TRANSLATE_NOOP("InstrumentsXML", "Chord symbols");
Copy link
Contributor Author

@Jojo-Schmitz Jojo-Schmitz May 10, 2020

Choose a reason for hiding this comment

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

Using the same context as the rest of the instruments, like long- and short names, channels, etc..
Simplifies the code that picks up those translations quite a bit, but means that the current translations of these 2 texts won't be taken (as they have a different context), so needs to get merged and translated to be able to see for sure whether it then shows the translated names in the UI.

Wonder whether to use QT_TRANSLATE_NOOP3("InstrumentsXML", "Normal", "Channel") instead to give some hint to the translators? Maybe modify share/instruments/generateTS.py to do the same for the channels from instruments.xml? Hmm, maybe not, better add some comments //: ... for the translators, I'll go with that.

Another and more important question is whether to use "normal" (lowercase) and "harmony" for those channel names (only the latter is used in the score files as far as I can tell), as in the original implementation (prior to efcf14a), and leave it to the translators to translate it for the UI (others like arco, pizzicato, etc. are lowercase too), esp. as "Chord symbols" is too long for that mixer label already.
Done now, via a 2nd commit.

@anatoly-os anatoly-os merged commit d52afdf into musescore:3.x May 12, 2020
anatoly-os added a commit that referenced this pull request May 12, 2020
fix #305134: crash on chordsymbol playback
@Jojo-Schmitz Jojo-Schmitz deleted the harmony-name branch May 12, 2020 06:09
item->setText(0, qApp->translate("InstrumentsXML", a->name().toUtf8().data()));
QString name = a->name();
if (a->name().isEmpty())
name == Channel::DEFAULT_NAME;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is supposed to be assignment, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Outch... Of course

}
QString name = e.name;
if (e.name.isEmpty())
name == Channel::DEFAULT_NAME;
Copy link
Contributor

Choose a reason for hiding this comment

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

And here?

}
QString name = e.name;
if (e.name.isEmpty())
name == Channel::DEFAULT_NAME;
Copy link
Contributor

Choose a reason for hiding this comment

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

And here?

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented May 12, 2020

Ok that is pretty bad. Thanks for noticing! Something to fix, tomorrow...

@Jojo-Schmitz
Copy link
Contributor Author

See #6082

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.

3 participants