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

[Thomas McGuire]

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


What happens in your case if "-- " is contained in the mail body somewhere and not meant as a separator? Like in "And then I did this -- can you believe it!?". I think you should only accept "-- " as a signature separator if it is either at the start of the mail or within a quoted text (quoted text: only >, | and ' ' before the signature)

The test code you wrote is an ideal candidate for writing unit tests for. Since the code is not trivial, please add a small unit tests, with those manual tests you did and the "-- in body" case I described above.

See http://techbase.kde.org/Development/Tutorials/Unittests.
Also see the utiltest and mimelibtest of KMail as an example on how to write tests.

Now, please change some coding style:
2 spaces indentation, no tabs please
Spaces inside parenthesis and around operators
No variable names with underscores (_), use camelCase instead
0!=prefix.size() => !prefix.isEmpty()
do .. while true => forever
next_prefix=""; => nextPrefix.clear()
"\n" => '\n'

- Thomas


On 2009-03-23 09:18:28, Jaime Torres wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/403/
> -----------------------------------------------------------
> 
> (Updated 2009-03-23 09:18:28)
> 
> 
> 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 940552 
> 
> Diff: http://reviewboard.kde.org/r/403/diff
> 
> 
> Testing
> -------
> 
> If checked with the following code:
> const QString mail="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"\
>                    "text5\n-- \nSignature Block 5\n";
> const QString mail1="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\n";
>                    
> const QString mail2="Text 1\n-- \nFirst sign\n\n\n> From: bla\n"\
> "> Texto 2\n\n> Aquí algo de texto.\n\n> --\n> Signature Block 2\n\n> Adios\n"\
> "\n>> Texto 3\n\n>> --\n>> Signature block 3\n";
> 
> const QString mail3="-- \n-- ACME, Inc\n-- Joe User\n-- PHB\n-- Tel.: 555 1234\n--";
> 
> 	QString res=stripSignature(mail,1);
> 	cout<<"res:"<<res.toStdString()<<endl;
> 	res=stripSignature(mail1,1);
> 	cout<<"res:"<<res.toStdString()<<endl;
> 	res=stripSignature(mail2,1);
> 	cout<<"res:"<<res.toStdString()<<endl;
> 	res=stripSignature(mail3,1);
> 	cout<<"res:"<<res.toStdString()<<endl;
> 
> 
> 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/