[ wxwindows-Patches-1881093 ] File/dir picker detail (CreatePicker)

[SourceForge.net]

Patches item #1881093, was opened at 2008-01-28 12:41
Message generated for change (Comment added) made by troelsk
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1881093&group_id=9863

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Common
Group: None
Status: Closed
Resolution: Accepted
Priority: 5
Private: No
Submitted By: troelsk (troelsk)
Assigned to: Nobody/Anonymous (nobody)
Summary: File/dir picker detail (CreatePicker)

Initial Comment:
Patch:
- Make it clear that CreatePicker functions are virtual
- Move CreatePicker implementations to cpp domain

Note:
- Blind patch. Trunk is not compiling currently (VC6 compilation, again broken by VS, movable.h)


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

>Comment By: troelsk (troelsk)
Date: 2008-02-03 16:38

Message:
Logged In: YES 
user_id=1290727
Originator: YES


>However I didn't move the functions out of line (why?)

In traditional non-templatized C++, implementation goes to cpp, not h,
simple as that. 
Unless I'm much mistaken, wx is indeed overwhelmingly traditional in this
respect.
Only not in filepicker.h apparantly. Disagreement is so much fun.




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

Comment By: Vadim Zeitlin (vadz)
Date: 2008-02-03 15:58

Message:
Logged In: YES 
user_id=71618
Originator: NO

We do prefer to repeat "virtual" in the derived classes to make it clear
that the functions are indeed virtual so I did add the two missing virtual
keywords. However I didn't move the functions out of line (why?) and I'd
also like to ask troelsk to stop calling people trolls without any reason,
personally I'm very glad that other people review the patches and leave
comments about them and I sincerely hope that Marcin continues to do it.

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

Comment By: troelsk (troelsk)
Date: 2008-01-31 10:54

Message:
Logged In: YES 
user_id=1290727
Originator: YES

>changing wxTextCtrl* to wxTextEntry* in pickerbase.h

Bad idea, wxPickerBase is too closely coupled with wxTextCtrl
(wxPickerBase::CreateBase m_text->Connect), don't touch pickerbase.h this
time around:
File Added: createpicker-virtual.patch

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

Comment By: troelsk (troelsk)
Date: 2008-01-29 07:48

Message:
Logged In: YES 
user_id=1290727
Originator: YES


Uploaded again with a small change. 

In the future wxFileDirPickers and friends will hopefully support
wxComboBox buddies (as well as wxTextCtrl). 
As a start: changing wxTextCtrl* to wxTextEntry* in pickerbase.h

Sorry about patching blind, but trunk still isn't compiling and I need to
leave this subject now.
(When the first [compiling] 2.9 file release arrives, testing patches
across platforms before uploading will become much easier: good line
endings!
http://sourceforge.net/tracker/index.php?func=detail&aid=1853440&group_id=9863&atid=359863)

File Added: createpicker-virtual.patch

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

Comment By: Marcin Wojdyr (wojdyr)
Date: 2008-01-28 16:05

Message:
Logged In: YES 
user_id=586843
Originator: NO

"You can use the virtual keyword in the derived-class declarations (it
does no harm to do so), 
but it is redundant and can be confusing."
You may disagree with this sentence, but anyway these functions are
virtual and you only changed coding style.
And don't call me troll.

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

Comment By: troelsk (troelsk)
Date: 2008-01-28 15:36

Message:
Logged In: YES 
user_id=1290727
Originator: YES


>this patch only changes coding style
Not at all. Please stop trolling.
Montorsi just forgot "virtual" in two places.


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

Comment By: Marcin Wojdyr (wojdyr)
Date: 2008-01-28 15:21

Message:
Logged In: YES 
user_id=586843
Originator: NO

-1
this patch only changes coding style, 
imho it's better how it is now


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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1881093&group_id=9863