D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:0b58dc656d1a: make units and prefixes of formatValue 
translatable (authored by astippich).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15829?vs=43038=46981

REVISION DETAIL
  https://phabricator.kde.org/D15829

AFFECTED FILES
  src/lib/util/kformatprivate.cpp

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich added a comment.


  If no-one objects or has any comments regarding coding, I'm going to land 
this revision in one week

REPOSITORY
  R244 KCoreAddons

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich added a comment.


  ping

REPOSITORY
  R244 KCoreAddons

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich updated this revision to Diff 43038.
astippich added a comment.


  - fixup spaces

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15829?vs=43034=43038

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

AFFECTED FILES
  src/lib/util/kformatprivate.cpp

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich added a comment.


  In D15829#338296 , @safaalfulaij 
wrote:
  
  > Thanks!
  >  I think this solved all my issues. I can for meter unit change the 
formatting so that `tr("%1 %2", "no Prefix")` is translated to `"%1%2"` and 
choose to keep the dot or remove it in the `unitString`s translation. (`"bit"` 
-> `".bit"`, `"m"` -> `"m"`)
  >  By the way, shouldn't that also be the case for English? I don't see 
people write `"1 m"` but `"1m"`, `"200 B"` but `"200B"`, same for `Hz`, not 
sure about `bit`s. The space is not there when abbreviation is used.
  
  
  Great! Now looking forward to feedback from @bruns regarding the 
implementation :)
  Scientifically it is correct to put a (small) space between value and unit.

REPOSITORY
  R244 KCoreAddons

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Safa Alfulaij]

safaalfulaij added inline comments.

INLINE COMMENTS

> kformatprivate.cpp:124
> +{ KFormat::UnitPrefix::Milli, 1e-3,  bpow(-10), tr("m", "SI prefix 
> for 10^⁻3") , QString() },
> +{ KFormat::UnitPrefix::Unity, 1.0,   1.0  , QString()
>   , QString()},
> +{ KFormat::UnitPrefix::Kilo,  1e3,   bpow(10) , tr("k", "SI prefix 
> for 10^3")  , tr("Ki", "IEC binary prefix for 2^10")},

Space before `}`?

> kformatprivate.cpp:125
> +{ KFormat::UnitPrefix::Unity, 1.0,   1.0  , QString()
>   , QString()},
> +{ KFormat::UnitPrefix::Kilo,  1e3,   bpow(10) , tr("k", "SI prefix 
> for 10^3")  , tr("Ki", "IEC binary prefix for 2^10")},
> +{ KFormat::UnitPrefix::Mega,  1e6,   bpow(20) , tr("M", "SI prefix 
> for 10^6")  , tr("Mi", "IEC binary prefix for 2^20") },

Space before `}`?

> kformatprivate.cpp:127
> +{ KFormat::UnitPrefix::Mega,  1e6,   bpow(20) , tr("M", "SI prefix 
> for 10^6")  , tr("Mi", "IEC binary prefix for 2^20") },
> +{ KFormat::UnitPrefix::Giga,  1e9,   bpow(30) , tr("G", "SI prefix 
> for 10^9")  , tr("Gi", "IEC binary prefix for 2^30")  },
> +{ KFormat::UnitPrefix::Tera,  1e12,  bpow(40) , tr("T", "SI prefix 
> for 10^12") , tr("Ti", "IEC binary prefix for 2^40") },

Extra space after `}`?

REPOSITORY
  R244 KCoreAddons

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Safa Alfulaij]

safaalfulaij accepted this revision.
safaalfulaij added a comment.
This revision is now accepted and ready to land.


  Thanks!
  I think this solved all my issues. I can for meter unit change the formatting 
so that `tr("%1 %2", "no Prefix")` is translated to `"%1%2"` and choose to keep 
the dot or remove it in the `unitString`s translation. (`"bit"` -> `".bit"`, 
`"m"` -> `"m"`)
  By the way, shouldn't that also be the case for English? I don't see people 
write `"1 m"` but `"1m"`, `"200 B"` but `"200B"`, same for `Hz`, not sure about 
`bit`s. The space is not there when abbreviation is used.

REPOSITORY
  R244 KCoreAddons

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich updated this revision to Diff 43034.
astippich added a comment.


  - add seperate, translatable string for IEC binary prefixes

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15829?vs=42563=43034

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

AFFECTED FILES
  src/lib/util/kformatprivate.cpp

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich added inline comments.

INLINE COMMENTS

> safaalfulaij wrote in kformatprivate.cpp:168
> Well, I searched other languages and no one seems to do as Arabic does. We do 
> not abbreviate in this case and just write the whole "Mebi.Byte" ("Byte" is 
> always written, "M.Byte" > "MB").
> And of course, there is no one solid naming of the units, so adding just one 
> letter will not get the right meaning. Adding another dot will make things 
> more crowded -> "M.i.Byte" (This is fine in English, but in Arabic it is 
> really ugly and means nothing.)

I uploaded a new version, but with what you describe, this will not suffice to 
fulfill your needs for IEC binary prefixes (please confirm). For the record, I 
haven't seen it anywhere in English that dots are put in between.
I guess a second map with the IEC binary prefixes is required so that one can 
translate them separately.

I'm away for the next week, so will continue to work on the patch afterwards.

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich updated this revision to Diff 42563.
astippich added a comment.


  - change description for prefixes and make the IEC prefix translatable

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15829?vs=42542=42563

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

AFFECTED FILES
  src/lib/util/kformatprivate.cpp

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Safa Alfulaij]

safaalfulaij added a comment.


  I came into anther problem, this time with the unit selection.
  In Arabic, we don't put a dot between the prefix and the unit if we're 
talking about meters (mm, cm, km), but we do with other units (k.Byte, M.Byte, 
G.Byte).
  
  If ki18n was used here, all of this can be locale-specific, but unfortunately 
that's not the case. I think I understand now why there are many strings for 
all of this unit stuff.

INLINE COMMENTS

> astippich wrote in kformatprivate.cpp:168
> Overlooked that one. Must this then be something like tr("1%2%).arg(... ...)?

Well, I searched other languages and no one seems to do as Arabic does. We do 
not abbreviate in this case and just write the whole "Mebi.Byte" ("Byte" is 
always written, "M.Byte" > "MB").
And of course, there is no one solid naming of the units, so adding just one 
letter will not get the right meaning. Adding another dot will make things more 
crowded -> "M.i.Byte" (This is fine in English, but in Arabic it is really ugly 
and means nothing.)

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich added a comment.


  In D15829#333799 , @safaalfulaij 
wrote:
  
  > Since the joining between number and format is translatable, that's great. 
I meant that if it is kept untranslatable, problems regarding RTL will happen.
  
  
  Sorry, I don't understand clearly, will this patch (once all issues are 
fixed) solve your problem or not?

INLINE COMMENTS

> safaalfulaij wrote in kformatprivate.cpp:115
> Line 160:
> 
>   } else {
>   value /= entry->binaryFactor;
>   prefixString = QString(entry->prefixChar).toUpper();
> 
> How this is “Metric”?

The prefixes for the IEC binary prefixes (for e.g. 2^10) are made up by the SI 
prefixes plus an "i", see below. But it actually is an SI prefix, to be more 
precise.

> safaalfulaij wrote in kformatprivate.cpp:168
> And what about this? //It should never be a puzzle string by the way.//

Overlooked that one. Must this then be something like tr("1%2%).arg(... ...)?

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Safa Alfulaij]

safaalfulaij added a comment.


  Since the joining between number and format is translatable, that's great. I 
meant that if it is kept untranslatable, problems regarding RTL will happen.

INLINE COMMENTS

> kformatprivate.cpp:115
>  const PrefixMapEntry map[] = {
> -{ KFormat::UnitPrefix::Yocto, 1e-24, bpow(-80), u'y' },
> -{ KFormat::UnitPrefix::Zepto, 1e-21, bpow(-70), u'z' },
> -{ KFormat::UnitPrefix::Atto,  1e-18, bpow(-60), u'a' },
> -{ KFormat::UnitPrefix::Femto, 1e-15, bpow(-50), u'f' },
> -{ KFormat::UnitPrefix::Pico,  1e-12, bpow(-40), u'p' },
> -{ KFormat::UnitPrefix::Nano,  1e-9,  bpow(-30), u'n' },
> -// Thanks to broken MSVC, we can not use u'µ', but have to use the 
> unicode codepoint
> -{ KFormat::UnitPrefix::Micro, 1e-6,  bpow(-20), QChar(0xB5) },
> -{ KFormat::UnitPrefix::Milli, 1e-3,  bpow(-10), u'm' },
> -{ KFormat::UnitPrefix::Unity, 1.0, 1.0, u'\0' },
> -{ KFormat::UnitPrefix::Kilo,  1e3,   bpow(10), u'k' },
> -{ KFormat::UnitPrefix::Mega,  1e6,   bpow(20), u'M' },
> -{ KFormat::UnitPrefix::Giga,  1e9,   bpow(30), u'G' },
> -{ KFormat::UnitPrefix::Tera,  1e12,  bpow(40), u'T' },
> -{ KFormat::UnitPrefix::Peta,  1e15,  bpow(50), u'P' },
> -{ KFormat::UnitPrefix::Exa,   1e18,  bpow(60), u'E' },
> -{ KFormat::UnitPrefix::Zetta, 1e21,  bpow(70), u'Z' },
> -{ KFormat::UnitPrefix::Yotta, 1e24,  bpow(80), u'Y' },
> +{ KFormat::UnitPrefix::Yocto, 1e-24, bpow(-80), tr("y", "Metric 
> prefix for 10^⁻24") },
> +{ KFormat::UnitPrefix::Zepto, 1e-21, bpow(-70), tr("z", "Metric 
> prefix for 10^⁻21") },

Line 160:

  } else {
  value /= entry->binaryFactor;
  prefixString = QString(entry->prefixChar).toUpper();

How this is “Metric”?

> kformatprivate.cpp:168
>  if (dialect == KFormat::IECBinaryDialect) {
>  prefixString += u'i';
>  }

And what about this? //It should never be a puzzle string by the way.//

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich added a comment.


  @bruns please have a look at the changes. In particular, regarding "µ" and 
windows support. I don't know if this will be able to compile on windows, or 
how to fix it for tr() function.
  @safaalfulaij please verify if this fixes the RTL issue for you

REPOSITORY
  R244 KCoreAddons

REVISION DETAIL
  https://phabricator.kde.org/D15829

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15829: make units and prefixes of formatValue translatable

[Alexander Stippich]

astippich created this revision.
astippich added reviewers: bruns, safaalfulaij.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  give the possibility to translate the prefix und unit strings

REPOSITORY
  R244 KCoreAddons

BRANCH
  translate_units

REVISION DETAIL
  https://phabricator.kde.org/D15829

AFFECTED FILES
  src/lib/util/kformatprivate.cpp

To: astippich, bruns, safaalfulaij
Cc: kde-frameworks-devel, michaelh, ngraham, bruns