<table><tr><td style="">gregormi marked an inline comment as done.<br />gregormi added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D7087">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7087#inline-67931">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Module.cpp:246</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">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.</p>

<p style="padding: 0; margin: 8px;">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.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D7087#inline-68264">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ltoscano</span> wrote in <span style="color: #4b4d51; font-weight: bold;">Module.cpp:253</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">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.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I will do it like this:</p>

<p style="padding: 0; margin: 8px;">i18nc("one line in the information that goes to the clipboard", "%1 %2", ...</p>

<p style="padding: 0; margin: 8px;">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".</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R102 KInfoCenter</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D7087">https://phabricator.kde.org/D7087</a></div></div><br /><div><strong>To: </strong>gregormi, ngraham, dhaumann<br /><strong>Cc: </strong>rkflx, dhaumann, ltoscano, sebas, elvisangelaccio, cfeck, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart<br /></div>