On 03/13/12 11:37, Mildred Ki'Lya wrote: > The patches are available at my branch called "clean":
Applied, thanks!
> Please check the fixes #8 has been added to the right commit
Looks good.
> - The TagListWidget shall be hidden when there's no visible message
>
> I did that, but I'm not sure it works. Please check.
Checked, works fine.
> - Gui/SettingsDialog.cpp's OutgoingPage constructor uses magic numbers
> for populating a combobox -- yes, that's my code, but I'd say that it
> must be possible to do that in some better form? Or mayeb not, please
> feel free to ignore this point :)
>
> I don't know what you're talking about, so i'm ignoring this point
I was speaking about this code, which IMHO looks ugly:
> method->insertItem( 0, tr("TCP"), QVariant( TCP ) );
> method->insertItem( 1, tr("SSL"), QVariant( SSL ) );
> method->insertItem( 2, tr("Local Process"), QVariant( PROCESS ) );
> using Common::SettingsNames;
> if ( QSettings().value( SettingsNames::imapMethodKey ).toString() ==
> SettingsNames::methodTCP ) {
> method->setCurrentIndex( 0 );
> } else if ( QSettings().value( SettingsNames::imapMethodKey ).toString()
> == SettingsNames::methodSSL ) {
> method->setCurrentIndex( 1 );
> } else {
> method->setCurrentIndex( 2 );
> }
...but as it's my code, I shall probably not complain too much.
> I'll let you choose the license.
If you're fine with the BSD license, I'd go for that.
> There is just FlowLayout that I copied from the Qt examples, I hope you
> don't mind me doing that.
It's all right.
> - Strings shall be either wrapped in tr() for user-facing ones or in
> QLatin1String() or QString::fromAscii() for those which are "magic"
> (like the IMAP flag names)
>
>
> I changed that, I didn't see any place where QLatin1String() or
> QString::fromAscii() was necessary though.
There's an optional macro QT_NO_CAST_FROM_ASCII which -- if defined --
disables constructing QStrings directly from string literals or char *.
The idea is to make sure that any user-facing strings either came
through tr() and are therefore translatable, or that the programmer made
an explicit effort indicating that a conversion shall take place (by
using QLatin1String, the preferred lightweight form, or through stuff
like QString::fromAscii). The end goal is to make sure that the
application can be properly translated and no English strings remain in
the UI.
Trojita cannot be compiled in this mode yet, but I do try to make sure
that the new code is "safe" in this regard. In practice, it means that:
- Any user-facing strings are wrapped in tr()
- QStrings are initialized either through QLatin1String("some literal"),
or via QString::fromAscii() (including the implicit conversions from
QByteArray)
That's what I'd like to have in all new code, but it isn't a real
problem at this point.
> - What do you think about providing localized names for some of the
> well-known flags like \Seen, \Deleted, $Forwarded etc etc?
>
> I thought that these special flags (starting with \ or $) should be
> hidden by default. But I did not implement that.
That'd be great -- if you have time in future, I'll be happy to merge a
patch doing something similar.
Cheers,
Jan
--
Trojita, a fast e-mail client -- http://trojita.flaska.net/
signature.asc
Description: OpenPGP digital signature
