[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request: Make mouse cursor size configurable
From: "Chusslove Illich" <caslav.ilic () gmx ! net>
Date: 2011-07-17 20:00:05
Message-ID: 20110717200005.5424.56873 () vidsolbach ! de
[Download RAW message or body]
> On July 12, 2011, 8:56 a.m., Christoph Feck wrote:
> > kcontrol/input/xcursor/xcursortheme.cpp, line 73
> > <http://git.reviewboard.kde.org/r/101701/diff/1/?file=24784#file24784line73>
> >
> > 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.
>
> Lukas Sommer wrote:
> 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.
>
> Chusslove Illich wrote:
> 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.
>
>
> Lukas Sommer wrote:
> 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?
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:".
- Chusslove
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101701/#review4633
-----------------------------------------------------------
On June 20, 2011, 10:04 a.m., Lukas Sommer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101701/
> -----------------------------------------------------------
>
> (Updated June 20, 2011, 10:04 a.m.)
>
>
> Review request for KDE Base Apps, KDE Runtime, kdelibs, and Christoph Feck.
>
>
> Summary
> -------
>
> 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.
>
> This addresses bug 90444.
> http://bugs.kde.org/show_bug.cgi?id=90444
>
>
> Diffs
> -----
>
> kcontrol/input/xcursor/xcursortheme.cpp a987487
> kcontrol/input/xcursor/themepage.ui 2e38054
> kcontrol/input/xcursor/xcursortheme.h b474086
> kcontrol/input/xcursor/themepage.h 902148f
> kcontrol/input/xcursor/themepage.cpp 24b9efe
> kcontrol/input/xcursor/previewwidget.h f4d2c4e
> kcontrol/input/xcursor/previewwidget.cpp 3c264fc
> kcontrol/input/xcursor/cursortheme.h 8f7834b
> kcontrol/input/xcursor/legacytheme.h 23c9d5f
>
> Diff: http://git.reviewboard.kde.org/r/101701/diff
>
>
> Testing
> -------
>
> Tested locally. Works fine for me. Also when using non-standard font DPI values.
>
>
> Screenshots
> -----------
>
>
> http://git.reviewboard.kde.org/r/101701/s/188/
>
> http://git.reviewboard.kde.org/r/101701/s/189/
>
>
> Thanks,
>
> Lukas
>
>
[Attachment #3 (text/html)]
<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>
</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, 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>
<br />
<p>- Chusslove</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>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic