[spambayes-bugs] [ spambayes-Patches-769981 ] Outlook plugin: allow user to change spam and unsure

[SourceForge.net]

Patches item #769981, was opened at 2003-07-12 02:48
Message generated for change (Comment added) made by xenogeist
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=498105&aid=769981&group_id=61702

Category: Outlook
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Adam Walker (xenogeist)
Assigned to: Mark Hammond (mhammond)
Summary: Outlook plugin: allow user to change spam and unsure

Initial Comment:
The config.py module does not allow a user to change 
the spam and/or unsure folders after changing them 
from the default of "None". The bug usually shows up 
after someone has deleted the spam folder by accident 
and requires the user to delete the ini file from the 
spambayes data folder as a workaround. However, the 
bug affects changing the values from existing folders as 
well.

The patch changes the is_valid_single method to test 
the "self.value" field which is a tuple in the case of spam 
and unsure folders instead of the "value" parameter 
which is a string in those cases. It will also return true 
in the case of a list so that the list of "good email" 
folders can be set.

The command used to create the attached diff was:
cvs diff -c config.py > config.diff

If I should do something differently, let me know.

----------------------------------------------------------------------

>Comment By: Adam Walker (xenogeist)
Date: 2003-07-20 06:51

Message:
Logged In: YES 
user_id=583713

Sorry, didn't realize the "multiple_values_allowed" method 
existed. Attached is a patch our mothers should be proud of ;)

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2003-07-20 06:11

Message:
Logged In: YES 
user_id=14198

I'm sorry, but I disagree.  The code is still taking the
incorrect path - the parent class still believes a
single-valued value is actually a multi-valued value.  Your
patch would indeed make it work, but is simply covering the
symptom of the bug rather than fixing it.

OK - how about this for an even better idea:
* Modify OptionsClass is_valid's if test to 'if
self.multiple_values_allowed():'
* Overide this single method in our class

That smells like the right fix to me!


----------------------------------------------------------------------

Comment By: Adam Walker (xenogeist)
Date: 2003-07-20 05:22

Message:
Logged In: YES 
user_id=583713

I agree with your summary of the OptionClass execute and 
that the first patch was flawed (I didn't have that case come 
up in testing). But I disagree with your suggested fix. I'm not 
a fan of dupicate code b/c it leads to maintaince problems. 
I'm attaching a new patch that leaves the is_valid_single 
method untoched but adds the "is_valid_multiple" method to 
the FolderIDOption class. This "is_valid_multiple" will not 
unpack tuples with a length of 2, but instead, pass the value 
to "is_valid_single". Any tuples whose length is not 2 or any 
value whose type is not a tuple will be processed by the 
Option.is_valid_multiple method.

----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2003-07-20 04:24

Message:
Logged In: YES 
user_id=14198

I don't think the patch is correct.  The most obvious
problem is that if self.value is None, any value is accepted.

The problem is that the base class is messing us up here. 
Consider the following execution order:

* Default options initialized with None
* Program attempts to set first value - tuple of (string,string)
* Base class is_valid called (OptionsClass.py).  This checks
if self.value is a list or tuple to determine if a
multi-valued option or not.  self.value currently None, so
not-multi.  Base class calls our is_valid_single.  This
passes, so option correctly set.

Now we attempt to set another folder - the process repeats.
* Base is_valid called.  self.value is now a tuple. 
is_valid incorrectly thinks this is now a multi-valued
option.  It therefore unpacks the existing value (ie, each
of the 2 strings in the ID) and passes this to
is_valid_single.  Hence, we get called with a string.

Note that in the "normal" multi-valued case, we expect a
list of these tuples.  Thus, as the list is unpacked, each
tuple is still correctly passed to is_valid_single, and
goodness prevails.

This is all my fault, and my bending of the options system
to my pre-existing "config" system.  The best fix I can some
up with is to a new "is_valid" method to the FolderIDOption
class, which is basically a clone of the parent method -
except that instead of checking:
        if type(self.value) in MultiContainerTypes:
you check:
        if type(self.default_value)==type([]):

I hope this makes sense.

----------------------------------------------------------------------

Comment By: Adam Walker (xenogeist)
Date: 2003-07-20 03:32

Message:
Logged In: YES 
user_id=583713

see the bug 761507 as an example of one that this patch 
may fix.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=498105&aid=769981&group_id=61702