Review Request: Make the 'Now listening..' presence string configurable

[Othmane Moustaouda]

> On March 18, 2012, 1:35 a.m., Daniele Elmo Domenichelli wrote:
> > Thanks for the patch, and welcome! A couple of comments:

4545


> On March 18, 2012, 1:35 a.m., Daniele Elmo Domenichelli wrote:
> > telepathy-mpris.cpp, line 114
> > <http://git.reviewboard.kde.org/r/104314/diff/1/?file=53621#file53621line114>
> >
> >     On the other hand we might want to do this i18n replacement only when the string is set and store the string always in this form, so that if one changes the language of the application it doesn't break...
> >     
> >     I'd consider making 3 replacements:
> >      - the first one from i18n("%title") to QLatin1String("%title") when we save the string from the KCM module
> >      - the second one from QLatin1String("%title") to the real title when nowPlaying replaces it (i.e. leave it unchanged here and modify the string where it is stored from the kcm module)
> >      - the third one from QLatin1String("%title") to i18n("%title") when you load the entry to display it in the KCM module.
> >     
> >     In this way we always store the english version but we display the i18n one.
> >     
> >     What do you think?
> 
> David Edmundson wrote:
>     That's frickin' clever.
>     
>     So to clarify we leave this code alone, sort out the translations purely in the config.
>     
>     So the user will type and see
>     "Ich höre zu %muziktitelsatzding" (or whatever the correct German is) and we will save "Ich höre zu %title"
>     
>     
>     
>     A big +1 from me, as that solves any issue with translators using different terms in the two places.
>     
>     QString nowPlayingText = kdedConfig.readEntry(QLatin1String("nowPlayingText"),
>                                        i18nc("The text displayed by now playing plugin", "Now listening to %1 by %2"), QLatin1String("%title"),                            QLatin1String("%artist"));
>     
>     and _then_ we do (which we'd be doing anyway)
>     
>     nowPlayingText.replace(QLatin1String("%title", i18n("%title")));
>     ui->setText(nowPlayingText);
>     
>     The translations part of "%title" is now only in one place, and not in two very different strings so it can't go wrong. (famous last words)
>     
>     
>     --
>     
>     Othmane does the suggestion make sense to you?

@Daniele
I think is a nice and "elegant" way to handle the i18n part, great idea!

@Dave
Thank you for the additional clarification :)
I understood the concept


> On March 18, 2012, 1:35 a.m., Daniele Elmo Domenichelli wrote:
> > telepathy-mpris.cpp, lines 115-116
> > <http://git.reviewboard.kde.org/r/104314/diff/1/?file=53621#file53621line115>
> >
> >     You might want to allow more fields here (track number, year, gender...) see http://www.mpris.org/2.1/spec/TrackList_Node.html#Mapping:Metadata_Map and http://xmms2.org/wiki/MPRIS_Metadata for the properties that you can set

Since the number of fields is enough consistent maybe it's better if I use a QStringList that contains all tags? Also i18n replacements managed via QStringList should be simpler...


- Othmane


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104314/#review11513
-----------------------------------------------------------


On March 17, 2012, 2:29 p.m., Othmane Moustaouda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104314/
> -----------------------------------------------------------
> 
> (Updated March 17, 2012, 2:29 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Enabled the possibility for the user to set his custom 'Now listening' presence string
> 
> 
> This addresses bug 282944.
>     http://bugs.kde.org/show_bug.cgi?id=282944
> 
> 
> Diffs
> -----
> 
>   config/telepathy-kded-config.cpp 1fb515f56500407b7b221d1d5310237f8d94ca7f 
>   telepathy-mpris.h d909863e68d0fdc7a6eb4ebb972d975ba224b6ef 
>   telepathy-mpris.cpp 8dae701a203f6c5ba43a622ce7125615b4392b63 
> 
> Diff: http://git.reviewboard.kde.org/r/104314/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Othmane Moustaouda
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120318/ac3ca663/attachment.html>