[Kde-pim] Review Request: Remove all the signature blocks.

[Ingo Klöcker]

> On 2009-03-25 13:23:24, Ingo Klöcker wrote:
> > The algorithm removes all lines ending with "-- ". This is wrong. It should only remove lines matching the regular expression "^[>| ]*-- $" and all following lines with the same prefix.
> > 
> > Moreover, you removed the case handling signed message text. Before a message is signed all trailing whitespace is removed. Therefore the signature separator loses the trailing space which is why the old code looks for "\n--\\s?\n", i.e. two dashes followed by an optional space, if clearSigned is true.
> > 
> > Splitting the message into lines and then joining the remaining lines at the end is very expensive if the message is large. Since all you do is remove lines, it would be best to do this in place. This way no additional memory would be needed. If the caller does not want the original text to be modified he can easily create a copy of the original text before calling stripSignature.
> 
> Jaime Torres wrote:
>     I'm guilty of removing the clearSign case. I'll add also comment about this case.
>     
>     A simple question. In kmail we can choose whatever we want as quote indicator, then Why only [>| ] ?
>     
>     I'll write also an inline version. And test it also with huge created mails.
>

You can add other common quote indicators if you like. [>| ] was meant as example. The important thing is that something like
>_>_bla_--_
(where _ stands for the space character) should not be removed.


- Ingo


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


On 2009-03-25 12:33:38, Jaime Torres wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/403/
> -----------------------------------------------------------
> 
> (Updated 2009-03-25 12:33:38)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Remove all the signature blocks.
> The algorithm is:
> Look for the prefixes before the "-- " marks (i.e. the text before it in the line).
> foreach line, if the line contains "-- ", start to delete lines until the prefix changes.
> 
> (it should also work with c code if the -- is not followed by a space).
> 
> 
> This addresses bug 72316.
>     https://bugs.kde.org/show_bug.cgi?id=72316
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/kmmessage.cpp 944557 
> 
> Diff: http://reviewboard.kde.org/r/403/diff
> 
> 
> Testing
> -------
> 
> #include <QtCore/QtCore>
> #include <iostream>
> 
> using namespace std;
> 
> static QString stripSignature ( const QString & msg, bool clearSigned )
> {
>   QRegExp SBDelimiterSearch = QRegExp( "-- $" );
>   QRegExp CommonReplySearch = QRegExp( "[>|']" );
>   
> 
>   // Split the string by line breaks
>   QStringList lines = msg.split( '\n' );
>   // Now, walk thougth the list again deleting from the "-- "$ until a prefix change.
>   // including empty prefixes and prefixes contained into other prefixes
> 
>   int i = 0;
> 
>   while ( i < lines.size() )
>   {
>     // look for the delimiter "-- "$
>     int posSignatureBlock = lines[i].indexOf( SBDelimiterSearch );
> 
>     if ( posSignatureBlock >= 0 )
>     {
>       // The prefix before "-- "$, for example, in ">> -- " is ">> "
>       const QString prefix = lines[i].left( posSignatureBlock );
>       lines.removeAt( i ); // removes the line "-- "$
> 
>       while ( i < lines.size() ) // lines srinks when a SB line is deleted
>       {
>         // remove the line of the Signature Block (SB)
>         if ( (lines[i].startsWith( prefix )) && (lines[i].indexOf( CommonReplySearch, posSignatureBlock) != posSignatureBlock ))
>           lines.removeAt( i );
>         else
>           break;
>       }
>     }
> 
>     ++i;
>   }
> 
>   // Join the lines and return the result.
>   return lines.join( "\n" );
> }
> 
> const QString testmail = "text1\n-- \nSignature Block1\nSignature Block1\n\n"\
>   "> text2\n> -- \n> Signature Block 2\n> Signature Block 2\n"
>   ">> text3 -- not a signature block\n>> text3\n"\
>   ">>> text4\n> -- \n> Signature Block 4\n> Signature Block 4\n"
>   ">>-------------\n>>-- text5 --\n>>-------------------\n>> -- \n>> Signature Block 5\n"
>   "text6\n-- \nSignature Block 6";
> const QString testmailExpected="text1\n"\
>   "> text2\n"
>   ">> text3 -- not a signature block\n>> text3\n"\
>   ">>> text4\n"
>   ">>-------------\n>>-- text5 --\n>>-------------------\n"
>   "text6";
> const QString testmail1 = "text1\n-- \nSignature Block1\nSignature Block1\n\n"
>   ">text2\n>-- \n>Signature Block 2\n>Signature Block 2\n"
>   "> >text3\n> >text3\n> >-- \n>>Signature Block 3\n> >Signature Block 3\n"
>   ">text4\n>-- \n>Signature Block 4\n>Signature Block 4\n"
>   "text5\n-- \nSignature Block 5";
> const QString testmail1Expected = "text1\n"
>   ">text2\n"
>   ">>Signature Block 3\n> >Signature Block 3\n"
>   ">text4\n"
>   "text5";
> // The first reply does not contains a signature block
> // but the second repy does.
> const QString testmail2 = "Text 1\n-- \nFirst sign\n\n\n> From: bla\n"
>   "> Texto 2\n\n> Aqui algo de texto.\n\n>> --\n>> Not Signature Block 2\n\n> Adios\n"
>   "\n>> Texto 3\n\n>> --\n>> Signature block 3\n";
> const QString testmail2Expected = "Text 1\n> From: bla\n"
>   "> Texto 2\n\n> Aqui algo de texto.\n\n>> --\n>> Not Signature Block 2\n\n> Adios\n"
>   "\n>> Texto 3\n\n>> --\n>> Signature block 3\n";
> 
> const QString testmail3 = "-- \n-- ACME, Inc\n-- Joe User\n-- PHB\n-- Tel.: 555 1234\n--";
> const QString testmail3Expected = "";
> 
> int
> main ( int argc, const char *argv[] )
> {
> 
>   QString res = stripSignature( testmail, 1 );
>   cout << "res:\n" << res.toStdString() << endl << "expected:\n" << testmailExpected.toStdString() << endl;
>   assert( res == testmailExpected);
>   res = stripSignature( testmail1, 1 );
>   cout << "res:\n" << res.toStdString() << endl << "expected:\n" <<  testmail1Expected.toStdString() << endl;
>   assert( res == testmail1Expected);
>   res = stripSignature( testmail3, 1 );
>   cout << "res:\n" << res.toStdString() << endl << "expected:\n" <<  testmail3Expected.toStdString() << endl;
>   assert( res == testmail3Expected);
>   res = stripSignature( testmail2, 1 );
>   cout << "res:\n" << res.toStdString() << endl << "expected:\n" <<  testmail2Expected.toStdString() << endl;
>   assert( res == testmail2Expected);
> 
> }
> 
> 
> Thanks,
> 
> Jaime
> 
>

_______________________________________________
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/