darktable
                                
                                 darktable copied to clipboard
                                
                                    darktable copied to clipboard
                            
                            
                            
                        Sentence case all UI strings
This is a follow up to, and a more radical version of #12243. It's a realization of the program proposed in https://github.com/darktable-org/darktable/issues/2078#issuecomment-749031332, as best as I could make it work.
All UI strings in Darktable are collected in the POT-file with source file and line. Roughly, there are seven categories of strings:
- Strings that are directly in the C sources. The source location should be correct for all of them.
- Strings that are sourced from $DESCRIPTIONcomments insrc/iopand are generated as part of the introspection mechanic. Unfortunately the POT file points us to generated sources in the build directory, but it's not hard to identify them and point them to the right file. However the line number is lost, so for these strings we have to search the whole file.
- Strings that are sourced from $DESCRIPTIONcomments in included header files. This constitutes another, more complicated layer of indirection, but there's not that many of them.
- Strings that are sourced from darktableconfig.xml.in. They also end up in one of two files in the build folder, and the POT file points to those generated files. Again, it's not hard to detect this and redirect to the right file, but, as before, line number information is lost so we have to search the whole file.
- Strings that are inserted into the generated config files from XSL during the build.
- Strings from the appdata file and the desktop file.
- The author section headers, which are generated during the build.
Using the source material from #12243 (huge thanks to @victoryforce who did the time consuming part of the job) I have manually updated strings of category 3, 5, 6 and 7, and I've written a relatively quick (but sufficiently robust) Python script to handle the remaining categories 1, 2 and 4, which are the vast majority of them.
One rule I followed for safety was that the original string and the replacement string MUST have the same length. Otherwise an error would be thrown. This rule is especially useful in the cases where long strings are split up into concatenated literals. This caused some friction that I had to fix:
- Inconsistent escaping of single quotes (apostrophes) in the C sources. This made me unable to pick a consistent style, because either the strings that had apostrophes escaped would fail, or the ones that didn't have them escaped would. So I manually went through and removed the escaped ones. As far as I know there's no reason to escape single quotes.
- Miscellaneous errors that had been fixed by @victoryforce in #12243 such as incorrect punctuation. In the majority of cases I agreed with their suggestion.
Additionally I found that in some modules the parameter name passed to dt_bauhaus_slider_from_params doubled as both an internal key and a user-facing string. This obviously broke, so I went through and fixed those instances, or at least the ones I could find. It's possible I missed some. In the future I recommend adding a $DESCRIPTION to every user-facing parameter.
Finally, the POT-file was refreshed and the msgids in all the PO-files were changed to the sentence cased strings. This was a raw fix: I didn't use the gettext merge tool. That means that the translations for the changed strings should be immediately usable without translator review. I believe the only backsliding for translation quality is the handful of manually changed strings.
The following table details the PO string substitutions. Where it says
M out of N replaced, K duplicates removed
It means that out of N entries in the PO-file, M were found to be an exact match for a lowercase string and were changed to a sentence cased string. In this process, K duplicates were found and removed (e.g. strings like "A4" and "a4" which were previously distinct but are now identical).
af.po          : 3519 out of 4701 replaced,  7 duplicates removed
ca.po          : 2185 out of 3268 replaced, 10 duplicates removed
cs.po          : 4588 out of 4787 replaced,  3 duplicates removed
da.po          : 2184 out of 3255 replaced, 11 duplicates removed
de.po          : 4588 out of 5867 replaced, 13 duplicates removed
el.po          : 2183 out of 3192 replaced,  4 duplicates removed
eo.po          : 4588 out of 4981 replaced,  5 duplicates removed
es.po          : 4588 out of 5913 replaced, 13 duplicates removed
fi.po          : 4588 out of 5410 replaced,  3 duplicates removed
fr.po          : 4590 out of 6034 replaced, 16 duplicates removed
gl.po          : 2183 out of 3325 replaced,  5 duplicates removed
he.po          : 4514 out of 5425 replaced,  3 duplicates removed
hu.po          : 4586 out of 5834 replaced, 13 duplicates removed
it.po          : 4588 out of 4615 replaced,  3 duplicates removed
ja.po          : 4590 out of 5532 replaced, 10 duplicates removed
nb.po          : 2183 out of 3099 replaced,  3 duplicates removed
nl.po          : 4588 out of 5835 replaced, 13 duplicates removed
pl.po          : 3961 out of 5422 replaced,  7 duplicates removed
pt_BR.po       : 4590 out of 4797 replaced,  4 duplicates removed
pt_PT.po       : 2183 out of 3239 replaced,  6 duplicates removed
ro.po          : 2183 out of 3233 replaced,  5 duplicates removed
ru.po          : 4588 out of 5814 replaced,  6 duplicates removed
sk.po          : 3026 out of 4003 replaced,  3 duplicates removed
sl.po          : 4588 out of 5484 replaced,  3 duplicates removed
sq.po          : 4590 out of 4616 replaced,  3 duplicates removed
sr.po          : 3168 out of 3871 replaced,  3 duplicates removed
[email protected]    : 3168 out of 3871 replaced,  3 duplicates removed
sv.po          : 3519 out of 4918 replaced, 12 duplicates removed
th.po          : 2183 out of 3108 replaced,  3 duplicates removed
tr.po          : 4588 out of 4765 replaced,  3 duplicates removed
uk.po          : 4590 out of 5021 replaced,  3 duplicates removed
zh_CN.po       : 4587 out of 5679 replaced,  8 duplicates removed
zh_TW.po       : 4590 out of 4616 replaced,  3 duplicates removed
I'm going to say it again... I don't think this change is necessary outside of tooltips, I don't think it adds anything particularly useful, and I'm not interested in spending weeks fixing dtdocs and requiring people to retranslate.
Well, keyboard shortcuts appear to be broken. Have to figure out how they work.
Also I'm pretty sure you've broken preferences, because while you've sentence-cased the options in darktableconfig.xml.in you've not sentence-cased the code that looks up those options.
Even if I thought this was feasible, there's only so far you'll get with automation. I think we'll spend years tidying up issues that weren't correctly automated. Also, IMO, Title Case would be preferable to sentence case for module names and widget controls.
This is a very high risk change (it touches nearly 180,000 lines, although much of it isn't "code") and I worry about the unseen breakages and how long we'll spend mopping up after. I don't mind sentence case / title case but I mind the amount of work/testing and the potential impact if mistakes are made.
@elstoc Sure, I understand. But consistent lowercase drives me sufficiently crazy that I'm willing to keep working on this, at least for the moment.
I'm not 100% sure but I believe the latest commit fixes the shortcuts.
I'm not 100% sure but I believe the latest commit fixes the shortcuts.
I'm not sure if "duplicate" presets/styles that differ only in capitalization are disallowed (at the moment I cannot test) but otherwise with this fix it would not be possible to assign them separate shortcuts.
You'll also need to change some strcmp in _shortcuts_load to strcasecmp, otherwise any existing shortcuts that specify elements or effects will likely break during the transition.
I'm willing to keep working on this, at least for the moment.
@elstoc 's point is that this would be inflicting the pain of catching and fixing additional bugs on everyone else too.
@elstoc 's point is that this would be inflicting the pain of catching and fixing additional bugs on everyone else too.
Yes and we can't protect dtdocs translators either (because weblate is more than just some po files) so again we're inflicting work on people who probably don't follow the gh repo that closely and therefore won't comment here. If I could wave my magic wand and be certain it would just work with no effort or risk I would be fine with it but IMO there are more important things to spend time on and this just isn't worth it.
On the whole, while not ideal, I prefer the idea of introducing a new "translation" that includes sentence case, because at least it's easy to disable and almost certain not to break stuff.
I prefer the idea of introducing a new "translation" that includes sentence case, because at least it's easy to disable and almost certain not to break stuff.
Preset and style names get internally stored and referred to in translated form, so could still cause breakage. Imho this is a suboptimal situation that would be nice to solve anyway (so that shortcuts don't get broken by changing languages) but non-trivial and so hasn't happened simply due to priorities and lack of time/motivation.
Preset and style names get internally stored and referred to in translated form, so could still cause breakage
Yes, this is an existing issue with changing language (I seem to recall Aurelien mentioning others as well) so as long as lowercase remains the default, at least it would still work out-of-the-box
- Comboboxes options don't need to be capitalized, only the labels do,
- Capitalizing labels in GUI doesn't need to change 172 000+ LoC, just grab the first letter of the labels and invert its casing.
- This PR has the potential to mess up a lot of things.
See https://github.com/darktable-org/darktable/pull/12243#issuecomment-1207068907 for the lightweight way.
This is going in the right direction, thanks a lot. Changing the case in all places seems a good start, actually that's the way it could have been done since the beginning. Lot of changes, but going in the right direction. That's certainly something I'm willing to merge sooner than later for 4.2 to ensure that possible issues are found while testing.
My only concern is about the shortcut actions, as @dterrahe as hinted and as you found out it is broken by this change. I'd like to have the actions also to be capitalized which also means that we want to gracefully update currently defined actions.
My only concern is about the shortcut actions
This would probably work correctly for fresh installs and only break old configurations during transition. And even that might be relative easily fixable (and tested!) with localized changes in accelerators.c.
Whereas the issue @elstoc highlights, that configuration options in xml.in have been changed, but not the code checking for the active setting, would need many changes all over the place.
@TheBB : You need to update the three newly merged translations. Hope this is not an issue for you? Let me know what's your plan on this as I'd like to merge sooner than later as it will really be impacted by many changes on the current code/translations. If updates are not a big issue (you can easily rework a new version of this PR using your script) then we are not in a hurry.
Updates are not difficult. I'm more concerned with figuring out all the other ways things can break.
On Tue, Aug 9, 2022, 22:43 Pascal Obry @.***> wrote:
@TheBB https://github.com/TheBB : You need to update the three newly merged translations. Hope this is not an issue for you? Let me know what's your plan on this as I'd like to merge sooner than later as it will really be impacted by many changes on the current code/translations. If updates are not a big issue (you can easily rework a new version of this PR using your script) then we are not in a hurry.
— Reply to this email directly, view it on GitHub https://github.com/darktable-org/darktable/pull/12281#issuecomment-1209865070, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXG3ZESTAN3HJZIGFEZLDVYK7GNANCNFSM55VMNARA . You are receiving this because you were mentioned.Message ID: @.***>
Updates are not difficult. I'm more concerned with figuring out all the other ways things can break.
I so think it's good to consider @aurelienpierre comment here and complement added, for doc, in #12243 related issue. His proposal seems a lot safer, simpler and lighter way to do that work. And I agree with all his 3 points here.
On reading here all feedback, I'm really worried by that PR (I completely agree with the goal but if the risk to mess up a lot of things as pointed by @elstoc, @aurelienpierre and @dterrahe, this is not going in the right direction that way. Sentence case is good, just be all sure this is done the best way and avoid add too much work to all involved people too).
I hope here that we will not make mix between the message and the messenger!
This pull request did not get any activity in the past 60 days and will be closed in 365 days if no update occurs. Please verify it has no conflicts with the master branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work.
I'm closing as we are moving toward a new translation, see #12243. @TheBB : Thanks for the work anyway.