[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: [PATCH] "Reply" button functionality in KMail (especially with
From:       Ingo =?iso-8859-15?q?Kl=F6cker?= <kloecker () kde ! org>
Date:       2007-11-27 23:27:16
Message-ID: 200711280027.20928 () erwin ! ingo-kloecker ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Saturday 24 November 2007, Matthew Blissett wrote:
> Hi
>
> (Some deleted quotation replaced)
>
> I've made a patch!  It's my first one, so let me know what can be
> improved. It applies to kdepim/kmail/kmmessage.cpp

The patch looks very good. Nevertheless I think there's room for 
improvement, in particular, with respect to readability:

> +        for ( QStringList::Iterator it = potentialTo.begin();
> +              it != potentialTo.end();
> +              ++it ) {
> +          if ( !addressIsInAddressList( *it, recipients ) ) {
> +            recipients += *it;
> +            kDebug(5006) << "Added" << *it << "to the list of
> recipients"; 
> +          }
> +        }

Please use a foreach-loop instead. If we touch this code anyway then we 
should also replace those ugly iterator-loops by the much better 
readable foreach-loops.


> +          for ( QStringList::Iterator it = potentialCC.begin();
> +                it != potentialCC.end();
> +                ++it ) {
> +            if ( !addressIsInAddressList( *it, recipients )
> +                 && !addressIsInAddressList( *it, ccRecipients ) ) {
> +              ccRecipients += *it;
> +              kDebug(5006) << "Added" << *it << "to the list of CC
> recipients"; 
> +            }
> +          }

Same here.


> +          for ( QStringList::const_iterator it =
> mailingListAddresses.begin(); 
> +                it != mailingListAddresses.end();
> +                ++it ) {
> +            recipients = stripAddressFromAddressList( *it,
> recipients ); 
> +          }

And here.


> +      toStr = ccRecipients[0];

Use the IMO better readable ccRecipients.first() instead. Also 
everywhere else where [0] is used.


> On Friday 23 November 2007 09:35:16 David Faure wrote:
> > On Thursday 22 November 2007 21:54:26 Ingo Klöcker wrote:
> > > On Wednesday 21 November 2007, Matthew Blissett wrote:
> > > > Hi
> > > >
> > > > I think this behaviour would be better:
> > > >
> > > > "Reply" button
> > > >   If "Folder holds a mailing list" is ticked for this folder
> > > >     - the same as "Reply to List" button
> > > >   Otherwise
> > > >     - the same as "Reply to Author" button
> > >
> > > I don't think this behavior is better than the current behavior.
> > > If anything then I'd change "Reply" to always do a reply to the
> > > Mail-Reply-To/Reply-To/From address.
>
> With the patch "Reply" will;
> if "Folder holds a mailing list" is ticked do the same as "Reply to
> list"; if not, do the same as "Reply to Author" button, except that
> it will honour a mailing-list-munged reply-to address.

I still don't like the reply-behavior depending on the "folder holds a 
mailing-list" setting. This is absolutely non-obvious. Moreover, I 
don't think many people set the "folder holds a mailing-list" setting 
because it is totally unnecessary for sane mailing lists, i.e. for 
mailing lists setting the List-Post header. All KDE mailing lists are 
sane.

For those reasons I object against this magic behavior. Either leave it 
as it is now or make "Reply" become an equivalent of "Reply to Author".

In fact, probably the best solution is to make the behavior of "Reply" 
configurable, i.e. it could either act like "Reply to Author" or 
like "Reply to List" (where the former would be the default and the 
latter would fallback to "Reply to Author" if "Reply to List" makes no 
sense for the replied-to message similar to the current behavior 
of "Reply"). This would allow us to get rid of the smart, but not easy 
to understand Reply and at the same time enable our users to retain the 
current behavior by changing the reply behavior from "to Author" to "to 
List".


> > Hmm, so after 8 years of pressing 'r' to reply on kde lists, it
> > would suddenly not reply to the list (except on those lists that
> > mangle Reply-To)? That would be pretty unexpected and confusing.

Reconsidering I agree with David (even though nowadays I always use 'l' 
for list-replies).


> Where there isn't a Reply-To header KMail previously would reply to
> the whole list.  With the patch, it will reply to the list if the
> folder is set as a mailing list.  Otherwise, it will reply to the
> From address (or Reply-To etc)
>
> When I forget to check the To: line and hit send, I'd prefer the safe
> option: that the message goes to one person when it was intended for
> many, rather than to many people when it was intended for one.

So you'd make "Reply" act like "Reply to Author".


> > At least if it depends on "Folder holds a mailing list" (which is
> > not in the RMB for folders so it's pretty hard to find BTW), it
> > will mean a lot of checkbox-clicking after upgrading, but at least
> > one will be able to set up kmail so that 'r' still replies to the
> > list (whatever the list configuration is). Yes, I do want to reply
> > on the list, 99.9999% of the time. When I want to mail a personal
> > reply I just do Shift+A, that's 0.0001% of the time...
>
> I get messages sent to "all-final-year-students@..." etc from staff
> at University, which aren't intended as discussion lists at all --
> Reply-To isn't set, and replies should go to the sender of the
> message. Currently, 'r' replies to the whole list, which is annoying.
> For real discussion mailing lists I can tick that box in the folder
> settings :-) (a RMB item would be good).

I'm sorry, but I'm getting the impression that you are trying to push a 
patch into KMail which fixes a problem only you are having. If you have 
a problem with those "all-final-year-students@..." messages because 
they have a List-Post header then why don't you simply remove the 
List-Post header from those messages with an appropriate filter?

[snip]


To summarize: I propose to get rid of the smart "Reply" and instead 
make "Reply" (configurablely) act either as "Reply to Author" or as an 
improved "Reply to Mailing-List" (which would basically act just as the 
smart "Reply" currently acts).


Regards,
Ingo

["signature.asc" (application/pgp-signature)]

_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel


[prev in list] [next in list] [prev in thread] [next in thread]