On Tue, 15 Oct 2002 22:31:05 +0200 (CET) Vadim Zeitlin <[EMAIL PROTECTED]> wrote:
> On Mon, 14 Oct 2002 17:41:11 -0700 (PDT) Michael A Chase > <[EMAIL PROTECTED]> wrote: > > MAC> I just uploaded my candidate patch to > MAC> http://home.ix.netcom.com/~mchase/M-641-mac-1.patch . It's about 38K, > MAC> so I wanted some comments before I sent it to the list or Bugzilla. > > Sorry as usual for the delay with replying -- OTOH you did fix a few bugs > apparently while I was going to reply to your previous message ;-) Thank you for waiting for me. :}b > # In include/MFilter.h: > # Add declarations for FilterTestImplemented(), FilterActionNeedsArg(), >FilterActionUsesColour(), > # FilterActionUsesFolder(), FilterActionMsgFlag(), and >FilterActionImplemented(). > > Is there any reason why the first and the last of those take an int and > not a corresponding enum? They all take an int instead of an enum, just like FilterTestNeedsArgument() and FilterTestNeedsTarget() which I copied them from. You are right though, they all ought to take the appropriate enum instead. I'll change them all. > # In src/gui/wxFilterDialog.cpp: > # Add comments giving enum symbols for ORC_Types[], ORC_Where[], ORC_Logical[], >and OAC_Types[]. > # Add ORC_Msg_Flag[] and OAC_Msg_Flag. > # Add checks for USE_PYTHON, USE_HEADER_SCORE. > # In OneCritControl:: > # Change GetArgument to include m_Msg_Flag. > # Add m_Msg_Flag. > > Could you please do something equivalent to > "%s/m_Msg_Flag/m_choiceFlags/g" > in your editor? I prefer to name the GUI controls with the prefix > corresponding to their type -- the existing code there doesn't do it and I > don't really want to change it but I'd like the new code to follow this > convention. No problem, I've changed it. > Very good. Thanks a lot for the detailed description of the changes, it > makes it so much easier to read the patch! It seemed like a good idea at the time. > MAC> I left the argument erase code in, but could remove it very easily. I > MAC> think hiding the controls is better than blotting out the string. > > I'll have to look at it to understand which is better. I think what you > did should be ok. No great rush. I plan to make some other changes in that general area after this patch goes in, so if you do decide to remove it, I can do it then. If the performance penalty isn't unacceptable and the change on the face of the dialog box isn't too jarring, I'd rather leave the value alone. I think preserving the value would make the dialog box easier to use. > There is no "Searched" flag, it's really a pseudo flag conjured out of > thin air by mail_search(). I don't think it makes sense to support it here. > OTOH, when we have support for the user-defined flags (something which most > IMAP servers can do and which could be implemented with the local files > quite easily), it would be nice to have support for them here -- but it's > for the future. I'll remove it for now. > Let me know if/when you're going to update the patch to incorporate the > minor changes I mentioned and I'll apply it (you can leave it on the web, > I'll fetch it from there). I'll finish the changes and let you know when I upload the updated the patch. I don't like the order of the choice lists being locked to the enum order. The enum values can't change because they've been writen in the saved filter rules, but nothing says the Selection() values have to be the same. If I add some arrays for translating forth and back it should be reasonably quick and more maintainable. It will also make leaving unimplemented choices out of the lists possible. I plan to submit a patch for that soon after this patch is applied to CVS. -- Mac :}) ** I normally forward private questions to the appropriate mail list. ** Ask Smarter: http://www.tuxedo.org/~esr/faqs/smart-questions.html Give a hobbit a fish and he eats fish for a day. Give a hobbit a ring and he eats fish for an age. ------------------------------------------------------- This sf.net email is sponsored by: viaVerio will pay you up to $1,000 for every account that you consolidate with us. http://ad.doubleclick.net/clk;4749864;7604308;v? http://www.viaverio.com/consolidator/osdn.cfm _______________________________________________ Mahogany-Developers mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/mahogany-developers
