use unicode × instead of ascii x for repeat counts#33625
Open
stevengj wants to merge 1 commit into
Open
Conversation
stevengj
force-pushed
the
sgj/repeat-times-symbol
branch
from
157a424 to
d0382af
Compare
Contributor
Author
|
Having some trouble building because of #33517 (comment) — looking for a workaround. Update: fixed by updating submodules (#33641) |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request replaces the ASCII letter 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai help |
Contributor
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
Visual tests are currently failing, but that's presumably expected because the rendering has changed. I should be able to update them once the build is fixed. |
Contributor
|
I have nothing against seeing this as an option, but I'd prefer it be offered as such (in the drop-down where you can select the preset. For one thing, while this symbol may be "widely" supported, it's not universal, so if nothing else there would need to be a fallback. And also, while the times sign may be preferred in some contexts, I don't think I've ever seen a published arrangement use it. |
Contributor
|
The unit tests need to get fixed, basically by changing the scores to what is expected IMHO no alternative option is needed, the new one looks much better. Then again my opinion doesn't count much... |
Contributor
Author
|
Let me know what is decided, and I will update the PR as needed. (Is there a (I think everyone involved in Musescore is in favor of good defaults. Here, it seems suboptimal to default to sloppy typography and include a "fix me" option. Tastes differ, of course, and neither "x" nor "×" may be widely established in music notation for repeat counts, but if "×3" is meant to denote "times 3" then the typography for the "times" symbol has been settled since the 17th century.) |
Contributor
|
There's no need to fix the vtest, it ain't do any good anyway. |
Contributor
Author
|
I don't really understand the test architecture (https://musescore.org/en/handbook/developers-handbook/finding-your-way-around/automated-tests seems somewhat out of date?) — where do I find the tests that need fixing? I looked at #28815, and the only test of the repeat count that I could see was |
Contributor
|
There's no need to update the translatione, that's done by the core team on a weekly basis. |
Contributor
|
This is the uniti teste that need fixing: |
Contributor
|
In short: don't care about the translations, it'll be done for you, don't care about the vtest failures, if, like in this case, they show what is to be expected, they will fix automagically once the PR has been merged, but do care about the utest failures, here by adjusting the mscx files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@share/locale/musescore_da.ts`:
- Line 29579: Update the three Danish translation strings that still use ASCII
"x" to use the Unicode multiplication symbol "×": change the translation
containing “x2” to “×2”, the one with “x3” to “×3”, and the one with “3x” to
“3×” in the musescore_da.ts locale entries (the lines containing the
translations "Vis for enkeltgentagelser (f.eks. “x2”)", the analogous "x3"
entry, and the "3x" entry).
In `@share/locale/musescore_de.ts`:
- Around line 29580-29592: The German translations still use ASCII "x" for the
repeat markers; update the translation strings corresponding to the source
entries "×3" and "3×" (from repeatplaycounttextmodel.cpp / the translation keys
shown) to use the Unicode multiplication symbol U+00D7 so they become "×3" and
"3×" (also ensure any other related translation like "×2" uses U+00D7 instead of
ASCII x).
In `@share/locale/musescore_en_GB.ts`:
- Around line 29583-29590: Translations for the two repeat play count entries
use ASCII "x" but should match the source Unicode multiplication sign "×";
update the translation strings corresponding to the source "×3" and "3×" (the
message entries from repeatplaycounttextmodel.cpp) to use "×3" and "3×"
respectively so typographic consistency is preserved.
In `@share/locale/musescore_en_US.ts`:
- Around line 29583-29589: Update the en_US translation entries that currently
read "x3" and "3x" to use the Unicode multiplication symbol so they exactly
match the source strings "×3" and "3×"; locate the translation entries that map
to those source strings (the pair where <source>×3</source> has
<translation>x3</translation> and where <source>3×</source> has
<translation>3x</translation>) and replace "x" with "×" in each <translation>
value.
In `@share/locale/musescore_en.ts`:
- Around line 25455-25456: The English translations still use ASCII "x" instead
of the Unicode multiplication sign "×" for the source strings like "×3" — update
the corresponding <translation> values (the entries with type="unfinished" that
currently contain "x3") to exactly match the source using the Unicode "×" (e.g.,
"×3"); also apply the same fix to the other occurrence noted (the entries around
the second instance) so all English translations mirror the source
multiplication symbol.
In `@share/locale/musescore_fo.ts`:
- Around line 29578-29585: The Faroese translations for the source strings "×3"
and "3×" in share/locale/musescore_fo.ts are still using ASCII "x" ("x3" and
"3x") and must either be updated to the Unicode multiplication sign to match the
source (change translations to "×3" and "3×") or be marked type="unfinished" so
translators can review; update the two <message> entries corresponding to the
source strings "×3" and "3×" (the one referencing repeatplaycounttextmodel.cpp)
accordingly.
In `@share/locale/musescore_it.ts`:
- Around line 29577-29589: The Italian translations in the message entries for
repeat play counts are using ASCII "x" instead of the Unicode multiplication
sign; update the translation values corresponding to the source strings "×2"
(example in the "Show for single repeats" message), "×3" (source in
repeatplaycounttextmodel.cpp), and "3×" to use "×2", "×3", and "3×" respectively
so they match the source symbols (look for the messages tied to
repeatplaycounttextmodel.cpp and the "Show for single repeats" source).
In `@share/locale/musescore_ko.ts`:
- Around line 29578-29590: Update the Korean translations to use the Unicode
multiplication symbol × instead of ASCII x for consistency with the source:
change the translation "단일 반복 시 표시 (예: \"x2\")" to "단일 반복 시 표시 (예: \"×2\")",
change the translation "x3" to "×3", and change "3x" to "3×" in the same message
entries corresponding to the repeatplaycounttextmodel strings.
In `@share/locale/musescore_pl.ts`:
- Around line 29578-29590: Update the Polish translations to use the Unicode
multiplication sign U+00D7 instead of ASCII "x" for the repeat indicators:
replace the translations corresponding to the source strings "Show for single
repeats (e.g. “×2”)" (translation currently "Pokaż dla pojedynczych repetycji
(np. „x2”)") and the literal sources "×3" and "3×" (translations currently "x3"
and "3x") so they read "Pokaż dla pojedynczych repetycji (np. „×2”)", "×3", and
"3×" respectively; ensure you update the translation values that match those
source texts in the same message entries so Polish locale displays the U+00D7
character consistently.
In `@share/locale/musescore_pt.ts`:
- Around line 29574-29581: Update the Portuguese translation entries that
currently use ASCII "x" to the Unicode multiplication sign: change the
translation value "x3" (for the source "×3") to "×3" and change "3x" (for the
source "3×") to "3×"; locate the two <message> blocks that contain the source
strings "×3" and "3×" (from repeatplaycounttextmodel.cpp) and replace their
translation text accordingly so they match the source typographically.
In `@share/locale/musescore_ro.ts`:
- Around line 29580-29591: Update the Romanian translations to use the Unicode
multiplication symbol U+00D7 instead of ASCII "x": change the translation for
the repeat example from „x2" to „×2", update the translation for the source "×3"
from x3 to ×3, and update the translation for the source "3×" from 3x to 3×;
these strings appear in share/locale/musescore_ro.ts corresponding to the source
lines in repeatplaycounttextmodel.cpp (line 45 and line 48) so edit those
translation entries to replace ASCII x with U+00D7.
In `@share/locale/musescore_sv_SE.ts`:
- Around line 29576-29588: The Swedish translations in
share/locale/musescore_sv_SE.ts are inconsistent with the source using the
multiplication symbol: update the three translated strings corresponding to the
repeatplaycounttextmodel entries so they use the Unicode multiplication sign
instead of ASCII x — change "x2" to "×2", "x3" to "×3", and "3x" to "3×" (these
map to the source strings "×2", "×3", "3×" from repeatplaycounttextmodel.cpp) so
the locale matches the typographically correct symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a93e309-41ad-4b4e-bb9a-19748d7cdde0
📒 Files selected for processing (74)
share/locale/musescore_af.tsshare/locale/musescore_ar.tsshare/locale/musescore_ar_DZ.tsshare/locale/musescore_ar_EG.tsshare/locale/musescore_ar_SD.tsshare/locale/musescore_ast.tsshare/locale/musescore_be.tsshare/locale/musescore_bg.tsshare/locale/musescore_br.tsshare/locale/musescore_ca.tsshare/locale/musescore_ca@valencia.tsshare/locale/musescore_cs.tsshare/locale/musescore_cy.tsshare/locale/musescore_da.tsshare/locale/musescore_de.tsshare/locale/musescore_el.tsshare/locale/musescore_en.tsshare/locale/musescore_en_GB.tsshare/locale/musescore_en_US.tsshare/locale/musescore_eo.tsshare/locale/musescore_es.tsshare/locale/musescore_et.tsshare/locale/musescore_eu.tsshare/locale/musescore_fa.tsshare/locale/musescore_fi.tsshare/locale/musescore_fil.tsshare/locale/musescore_fo.tsshare/locale/musescore_fr.tsshare/locale/musescore_ga.tsshare/locale/musescore_gd.tsshare/locale/musescore_gl.tsshare/locale/musescore_he.tsshare/locale/musescore_hi_IN.tsshare/locale/musescore_hr.tsshare/locale/musescore_hu.tsshare/locale/musescore_hy.tsshare/locale/musescore_id.tsshare/locale/musescore_ig.tsshare/locale/musescore_it.tsshare/locale/musescore_ja.tsshare/locale/musescore_ka.tsshare/locale/musescore_kab.tsshare/locale/musescore_ko.tsshare/locale/musescore_lt.tsshare/locale/musescore_lv.tsshare/locale/musescore_ml.tsshare/locale/musescore_mn_MN.tsshare/locale/musescore_mt.tsshare/locale/musescore_nb.tsshare/locale/musescore_nl.tsshare/locale/musescore_nn.tsshare/locale/musescore_pl.tsshare/locale/musescore_pt.tsshare/locale/musescore_pt_BR.tsshare/locale/musescore_ro.tsshare/locale/musescore_ru.tsshare/locale/musescore_scn.tsshare/locale/musescore_sk.tsshare/locale/musescore_sl.tsshare/locale/musescore_sr.tsshare/locale/musescore_sr_RS.tsshare/locale/musescore_sv.tsshare/locale/musescore_sv_SE.tsshare/locale/musescore_th.tsshare/locale/musescore_tr.tsshare/locale/musescore_uk.tsshare/locale/musescore_uz@Latn.tsshare/locale/musescore_vi.tsshare/locale/musescore_zh_CN.tsshare/locale/musescore_zh_HK.tsshare/locale/musescore_zh_TW.tssrc/notationscene/qml/MuseScore/NotationScene/styledialog/RepeatPage.qmlsrc/notationscene/qml/MuseScore/NotationScene/styledialog/repeatplaycounttextmodel.cppvtest/scores/play-count.mscz
✅ Files skipped from review due to trivial changes (17)
- share/locale/musescore_ru.ts
- share/locale/musescore_nn.ts
- share/locale/musescore_ar.ts
- share/locale/musescore_nl.ts
- share/locale/musescore_fa.ts
- share/locale/musescore_ig.ts
- share/locale/musescore_ar_EG.ts
- share/locale/musescore_et.ts
- share/locale/musescore_scn.ts
- share/locale/musescore_th.ts
- share/locale/musescore_hy.ts
- share/locale/musescore_hu.ts
- share/locale/musescore_ar_SD.ts
- share/locale/musescore_nb.ts
- share/locale/musescore_ja.ts
- share/locale/musescore_pt_BR.ts
- share/locale/musescore_cs.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- share/locale/musescore_lt.ts
- share/locale/musescore_uz@Latn.ts
- share/locale/musescore_ca@valencia.ts
- share/locale/musescore_sv.ts
- share/locale/musescore_zh_CN.ts
- share/locale/musescore_sr_RS.ts
- share/locale/musescore_sr.ts
- share/locale/musescore_be.ts
- share/locale/musescore_eu.ts
- share/locale/musescore_bg.ts
- share/locale/musescore_fil.ts
- share/locale/musescore_lv.ts
- share/locale/musescore_eo.ts
- share/locale/musescore_uk.ts
- share/locale/musescore_ast.ts
- share/locale/musescore_ka.ts
- share/locale/musescore_ca.ts
- share/locale/musescore_es.ts
- share/locale/musescore_tr.ts
- share/locale/musescore_sl.ts
- share/locale/musescore_fr.ts
- share/locale/musescore_fi.ts
Contributor
Author
|
It looks like I might need to update the translations, since some of the strings already have translations and possibly those translations will be lost if the translation files are regenerated with the new keys? (Though the translations I can find seem to be the identity translation, so maybe no loss?) |
Contributor
|
Those translations need to get redone, but just leave that to the translators. |
Contributor
Author
|
Thanks, done. |
Contributor
|
Now squash all these commits into just one... |
Contributor
Author
|
(I can do that — and did — but why not just squash when/if the PR is merged using github's squash-and-merge feature?) |
stevengj
force-pushed
the
sgj/repeat-times-symbol
branch
from
0466aa5 to
eea9ca8
Compare
Contributor
|
Because the developers doing the merge don't like it, as far as I can tell |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: #28815 (comment)
#28815 by @miiizen added the option to indicate repeat counts like "x3" or "3x" for repeat marks that use a count ≠ 2. However, the use of the ASCII letter "x" to represent "times", while common in informal writing (because it is easy to type), is usually considered poor typography — one should generally use the the Unicode multiplication symbol
×(U+00D7) instead, e.g. "×3" or "3×").(Note that the
×symbol U+00D7 has been part of Unicode since version 1.1 in 1993, and in fact dates back to Latin-1 from the 1980s, and seems to be widely supported by fonts — as far as I can determine, it is present in every modern general-purpose font for Western languages. e.g. see this incomplete list. Besides, doesn't the text rendering support the fallback fonts provided by every modern OS? Encoding support for × is literally older than that of curly quotes ala #17550, since curly quotes weren't included in Latin-1.)