gregormi marked an inline comment as done.
gregormi added inline comments.

INLINE COMMENTS

> rkflx wrote in Module.cpp:246
> Probably not strictly required for this patch, but it would be nicer to 
> refactor this in such a way that adding another string to the UI does not 
> require adding it here too.
> 
> Perhaps this can be achieved by creating a list of label/version pairs, and 
> then iterating through that list both when creating the UI and when 
> generating the text to copy.

Yes, I also had the list idea in an earlier stage of the patch< but without 
reusing the translated label text. I don't expect the information labels to 
change often, so I think such a refactoring can be done at a later point in 
time.

> ltoscano wrote in Module.cpp:253
> Please don't write (just) "do this in this case", but explain what the 
> placeholders are meant to be. Leave it to the speakers of the language the 
> decision about the order.

I will do it like this:

i18nc("one line in the information that goes to the clipboard", "%1 %2", ...

I see one problem: %1 contains a trailing colon (:). So just reversing to "%2 
%1" would result in "openSUSE Tumbleweed Distro:". One solution would be to 
strip the colon from the %1 string and put it here: "%1: %2".

REPOSITORY
  R102 KInfoCenter

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

To: gregormi, ngraham, dhaumann
Cc: rkflx, dhaumann, ltoscano, sebas, elvisangelaccio, cfeck, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, 
mart

Reply via email to