<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/101701/">http://git.reviewboard.kde.org/r/101701/</a>
     </td>
    </tr>
   </table>
   <br />








<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On July 12th, 2011, 8:56 a.m., <b>Christoph Feck</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="http://git.reviewboard.kde.org/r/101701/diff/1/?file=24784#file24784line73" style="color: black; font-weight: bold; text-decoration: underline;">kcontrol/input/xcursor/xcursortheme.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>

 <tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
  <tr>

   <td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">XCursorTheme::XCursorTheme(const QDir &themeDir)</pre></td>

  </tr>
 </tbody>




 
 



 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">73</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">          <span class="s">"The second argument is the list of available sizes, which is > 0"</span><span class="p">,</span></pre></td>
  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Do you mean the sizes are always > 0, or do you mean the number of entries in the list is > 0 ("never empty")?

Do we need some logic to prevent it saying "Available sizes:" when only one size is available? But I am not sure if i18n can handle that.</pre>
 </blockquote>



 <p>On July 17th, 2011, 8:26 a.m., <b>Lukas Sommer</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It means "never empty".

This could be a solution to the plural problem:

        // The translation is aware of plural forms. i18ncp uses the first integer argument to
        // distinguish plural forms. The first and the second argument are QString. So we use
        // sizeList.size() as third argument to provide proper support for plural forms. This 
        // works, although it is never referenced with %3 in the string itself. Although we
        // provide english strings only for "1 item" and for "more than 1 item", but i18ncp
        // will silently expand to as many plural forms as necesarry in the target language.
        m_description = i18ncp(
          "@info/plain This is the description of the cursor themes. The first argument is the "
            "original description that comes from the index.theme file. The second argument is "
            "the list of available sizes, which is never empty. This string is localized with "
            "support for plural forms: You can make different singular/plural translations "
            "depending on the number (!) of list items in the second argument.",
          "%1 (Size: %2)"            /* only 1 item in sizeListString      */
          "%1 (Available sizes: %2)" /* more than 1 item in sizeListString */
          ).arg(m_description).arg(sizeliststring).arg(sizeList.size());

I'll test this after holiday.</pre>
 </blockquote>





 <p>On July 17th, 2011, 11:02 a.m., <b>Chusslove Illich</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">i18ncp() should not be used for clever tricks. The plural string should
always be the direct pluralization of the singular string.

I think it would be just fine to leave it at the original version, with "...
Available sizes: ..." even if there is only one size. Also no need to
mention in the context that list of sizes is never empty.

Otherwise, use two separate messages with ordinary if-selection.
</pre>
 </blockquote>





 <p>On July 17th, 2011, 12:18 p.m., <b>Lukas Sommer</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">What's about merging both approaches?

        int sizeListSize = sizeList.size();
        QString sizeListString = QString::number(sizeList.takeFirst());
        while (!sizeList.isEmpty())
        {
            sizeListString.append(", ");
            sizeListString.append(QString::number(sizeList.takeFirst()));
        };
        if (sizeList.size() == 1)
            m_description = i18ncp(
                "@info/plain This is the description of the cursor themes. The first argument is "
                    "the original description that comes from the index.theme file. The second "
                    "argument is the size (in pixel). Example: 'OriginalDescription (Size: 24)'",
                "%1 (Size: %2)").arg(m_description).arg(sizeListString);
        else // sizeList.size() > 1
            /* The translation is aware of plural forms. i18ncp uses the first integer argument to
               distinguish plural forms. The first and the second argument are QString. So we use
               sizeList.size() as third argument to provide proper support for plural forms. This 
               works, although it is never referenced with %3 in the string itself. Although we
               provide english strings only for "1 item" and for "more than 1 item", but i18ncp
               will silently expand to as many plural forms as necesarry in the target language. */
            m_description = i18ncp(
                "@info/plain This is the description of the cursor themes. The first argument is "
                    "the original description that comes from the index.theme file. The second "
                    "argument is the list of available sizes (in pixel). This string is localized "
                    "with support for plural forms: You can make different singular/plural "
                    "translations depending on the number (!) of list items in the second "
                    "argument. Example: 'OriginalDescription (Available sizes: 24, 36, 48)' has "
                    "the plural form for 3 because the list has 3 items.",
                "%1 (Available size: %2)"  /* only 1 item in sizeListString - will never be used */
                "%1 (Available sizes: %2)" /* more than 1 item in sizeListString */
                ).arg(m_description).arg(sizeListString).arg(sizeListSize);

This way we have a good translation for only 1 size: (Size: 24). In this case the user can't do anything but use this size. And in the case of more than 1 size, the user can determine himself which size he wants to use: (Available sizes: 24. 36, 48)

Otherwise, we could also drop the "if" block and use only the "else" block ...

What do you think?</pre>
 </blockquote>





 <p>On July 17th, 2011, 7:59 p.m., <b>Chusslove Illich</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Plural messages really are to be used only when a part of the sentence
grammatically depends on a number, and that number itself is part of the
sentence. So there should not be any kind of plural message here.

>From the style viewpoint, I personally would stick with single "Available
sizes: ..." regardless of how many sizes there actually are. If a special
message is used for the case of size == 1, I have no clear preference
between "Size:" and "Available size:".
</pre>
 </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> Plural messages really are to be used only when a part of the
> sentence grammatically depends on a number

Well, I thought this depends on the number of items ;-) In singular: "Size: 24", but in plural "Sizes: 24, 36".

However, when you think that this is a bad solution, we can remove it and return to the original version (with a better comment):

        QString sizeListString = QString::number(sizeList.takeFirst());
        while (!sizeList.isEmpty())
        {
            sizeListString.append(", ");
            sizeListString.append(QString::number(sizeList.takeFirst()));
        };
        m_description = i18nc(
            "@info/plain This is the description of the cursor themes. The first argument is "
                "the original description that comes from the index.theme file. The second "
                "argument is the list of available sizes (in pixel). Example: 'OriginalDescription "
                "(Available sizes: 24)' or 'OriginalDescription (Available sizes: 24, 36, 48)'",
            "%1 (Available sizes: %2)").arg(m_description).arg(sizeListString);

Okay?</pre>
<br />




<p>- Lukas</p>


<br />
<p>On June 20th, 2011, 10:04 a.m., Lukas Sommer wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for KDE Base Apps, KDE Runtime, kdelibs, and Christoph Feck.</div>
<div>By Lukas Sommer.</div>


<p style="color: grey;"><i>Updated June 20, 2011, 10:04 a.m.</i></p>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">X11 mouse cursor themes can contain cursors in multiple sizes, making them pseudo-scalable.

It is yet possible in KDE to configure manually the mouse cursor size (editing kcminput.rc). However, the GUI of the corresponding KControl module didn't provide support to change this. This patch add support for changing the mouse cursor size to the GUI.

This are mostly GUI related changes. The underlying data structure XCursorTheme did yet provide support for choosing different sizes and only needed some adjustments.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested locally. Works fine for me. Also when using non-standard font DPI values.</pre>
  </td>
 </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=90444">90444</a>


</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kcontrol/input/xcursor/xcursortheme.cpp <span style="color: grey">(a987487)</span></li>

 <li>kcontrol/input/xcursor/themepage.ui <span style="color: grey">(2e38054)</span></li>

 <li>kcontrol/input/xcursor/xcursortheme.h <span style="color: grey">(b474086)</span></li>

 <li>kcontrol/input/xcursor/themepage.h <span style="color: grey">(902148f)</span></li>

 <li>kcontrol/input/xcursor/themepage.cpp <span style="color: grey">(24b9efe)</span></li>

 <li>kcontrol/input/xcursor/previewwidget.h <span style="color: grey">(f4d2c4e)</span></li>

 <li>kcontrol/input/xcursor/previewwidget.cpp <span style="color: grey">(3c264fc)</span></li>

 <li>kcontrol/input/xcursor/cursortheme.h <span style="color: grey">(8f7834b)</span></li>

 <li>kcontrol/input/xcursor/legacytheme.h <span style="color: grey">(23c9d5f)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/101701/diff/" style="margin-left: 3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>

<div>

 <a href="http://git.reviewboard.kde.org/r/101701/s/188/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/06/20/snapshot1_1_400x100.png" style="border: 1px black solid;" alt="" /></a>

 <a href="http://git.reviewboard.kde.org/r/101701/s/189/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/06/20/snapshot2_1_400x100.png" style="border: 1px black solid;" alt="" /></a>

</div>


  </td>
 </tr>
</table>








  </div>
 </body>
</html>