[Kde-pim] Review Request: Allow building with -DQT_NO_CAST_FROM_ASCII

[Ingo Klöcker]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2423/#review3464
-----------------------------------------------------------


The changes seem to be okay. Nevertheless, I suggest to use QString::fromLatin1() instead of QString::fromAscii() because it's usually faster. mbox files do not have a specific character encoding. They have to be treated as binary 8-bit text files with no particular encoding. The conversion to Unicode is done according to the charset information of the MIME body parts of the messages stored in the mbox file.

As QRegExp works on QStrings but not on QByteArrays it seems to be the wrong tool. Converting 8-bit text to Unicode just to be able to use QRegExp is questionable and slow.

All of those
  regexp.indexIn( QString::fromAscii( line ) ) >= 0
and
  regexp.indexIn( QString::fromAscii( line ) ) < 0
should be replaced by a call of a helper method, e.g.

static bool isMBoxSeparatorLine( const QByteArray& line )
{
  static QString sMBoxSeperatorRegExp( QLatin1String( "^From .*[0-9][0-9]:[0-9][0-9]" ) );
  if ( ! line.startsWith( "From " ) )
  {
    return false;
  }
  return sMBoxSeperatorRegExp.indexIn( QString::fromLatin1( line ) ) >= 0;
}


- Ingo


On 2009-12-19 16:15:20, Kevin Krammer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2423/
> -----------------------------------------------------------
> 
> (Updated 2009-12-19 16:15:20)
> 
> 
> Review request for KDE PIM and Bertjan Broeksema.
> 
> 
> Summary
> -------
> 
> Replaces implicit ByteArray to QString conversion with explicit QString::fromAscii(), QLatin1String for const char* literals, QString::fromLocal8Bit() for encoded filenames.
> 
> No idea whether mbox files are always ASCII, maybe one of the other "from" methods is more appropriate
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/runtime/resources/mbox/libmbox/tests/mboxbenchmark.cpp 1063924 
>   /trunk/KDE/kdepim/runtime/resources/mbox/libmbox/tests/mboxtest.cpp 1063924 
>   /trunk/KDE/kdepim/runtime/resources/mbox/libmbox/mbox.cpp 1063924 
> 
> Diff: http://reviewboard.kde.org/r/2423/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/