Re: #11088: Addition of wxDataViewListStore::Set/GetItemData
10 views
Skip to first unread message
wxTrac
Jul 21, 2011, 9:01:39 AM7/21/11
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:7>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------+---------------------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone:
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API | Blockedby:
Patch: 1 | Blocking:
------------------------------------+---------------------------------------
Changes (by vadz):
* priority: normal => low
* milestone: 3.0 =>
Comment:
Sorry for returning this so late but I finally did and now I have another
question: why are these functions added to `wxDataViewListStore` and not
`wxDataViewListCtrl` itself? If the goal is "compatibility" with
`wxListCtrl`, wouldn't it make more sense to put them in the latter?
Also, any new functions need to be documented (in
`interface/wx/dataview.h` in this case) and please avoid hard TABs in
wxWidgets sources if you update the patch.
TIA!
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:7>
wxTrac
Dec 29, 2011, 3:12:39 PM12/29/11
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:8>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone:
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Changes (by vadz):
* keywords: wxDataViewListCtrl API => wxDataViewListCtrl API docs-needed
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:8>
wxTrac
May 24, 2012, 8:32:07 PM5/24/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:9>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone:
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Changes (by Kry):
* cc: kry@… (added)
Comment:
I added the proper interface in wxDataViewListCtrl, mirroring the one in
wxDataViewTreeCtrl, and also added documentation mirroring the one for
wxDataViewTreeStore/wxDataViewTreeCtrl. No hard TABs.
I hope this is enough to get the patch included! It's really useful in
order to port code that used wxListCtrl.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:9>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone:
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
* cc: kry@… (added)
Comment:
I added the proper interface in wxDataViewListCtrl, mirroring the one in
wxDataViewTreeCtrl, and also added documentation mirroring the one for
wxDataViewTreeStore/wxDataViewTreeCtrl. No hard TABs.
I hope this is enough to get the patch included! It's really useful in
order to port code that used wxListCtrl.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:9>
wxTrac
May 25, 2012, 4:51:39 AM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:10>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
* milestone: => 3.0
Comment:
Thanks, the patch is almost ready now but there is one more change that
should be done: could you please use `wxUIntPtr` instead of
`wxClientData*`? The reason for insisting on this (other than just
backwards compatibility) is that `wxClientData` is supposed to be deleted
by the control but this is not the case here AFAICS. So using it would be
really confusing and either `void*` or, for better compatibility with
plain `wxListCtrl`, `wxUIntPtr` should be used.
Also, could you please use "@since 2.9.4" in the new methods
documentation?
TIA!
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:10>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Changes (by vadz):
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
* milestone: => 3.0
Comment:
Thanks, the patch is almost ready now but there is one more change that
should be done: could you please use `wxUIntPtr` instead of
`wxClientData*`? The reason for insisting on this (other than just
backwards compatibility) is that `wxClientData` is supposed to be deleted
by the control but this is not the case here AFAICS. So using it would be
really confusing and either `void*` or, for better compatibility with
plain `wxListCtrl`, `wxUIntPtr` should be used.
Also, could you please use "@since 2.9.4" in the new methods
documentation?
TIA!
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:10>
wxTrac
May 25, 2012, 6:22:33 AM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:11>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Comment(by Kry):
Hey Vadim,
Actually, just like wxDataViewTreeStoreNode, wxDataViewListStoreLine
deletes the client data on the destructor, which is called for all items
on ~wxDataViewListStore()
{{{
wxDataViewListStore::~wxDataViewListStore()
{
wxVector<wxDataViewListStoreLine*>::iterator it;
for (it = m_data.begin(); it != m_data.end(); ++it)
{
wxDataViewListStoreLine* line = *it;
delete line;
}
}
}}}
{{{
virtual ~wxDataViewListStoreLine()
{
delete m_data;
}
}}}
So AFAICS, the client data is deleted? We could always change it to not
delete the data, but then we'd have the same situation in
wxDataViewTreeCtrl, and some people may expect the data to be deleted (not
as much of a big deal as if it were NOT deleted and we changed it, tho).
Another thing I want to discuss it the need for a GetItemCount to mirror
the wxListCtrl original function, instead of forcing the developer to use
GetModel()->m_data.size(), which is both ugly and probably m_data
shouldn't be public to begin with.
I have implemented it as a simple
{{{
unsigned int GetItemCount() const;
}}}
With the proper documentation and everything. Can I just include it in
this patch as a general porting help for the transition to the new
classes?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:11>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Hey Vadim,
Actually, just like wxDataViewTreeStoreNode, wxDataViewListStoreLine
deletes the client data on the destructor, which is called for all items
on ~wxDataViewListStore()
{{{
wxDataViewListStore::~wxDataViewListStore()
{
wxVector<wxDataViewListStoreLine*>::iterator it;
for (it = m_data.begin(); it != m_data.end(); ++it)
{
wxDataViewListStoreLine* line = *it;
delete line;
}
}
}}}
{{{
virtual ~wxDataViewListStoreLine()
{
delete m_data;
}
}}}
So AFAICS, the client data is deleted? We could always change it to not
delete the data, but then we'd have the same situation in
wxDataViewTreeCtrl, and some people may expect the data to be deleted (not
as much of a big deal as if it were NOT deleted and we changed it, tho).
Another thing I want to discuss it the need for a GetItemCount to mirror
the wxListCtrl original function, instead of forcing the developer to use
GetModel()->m_data.size(), which is both ugly and probably m_data
shouldn't be public to begin with.
I have implemented it as a simple
{{{
unsigned int GetItemCount() const;
}}}
With the proper documentation and everything. Can I just include it in
this patch as a general porting help for the transition to the new
classes?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:11>
wxTrac
May 25, 2012, 6:43:09 AM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:12>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Comment(by vadz):
`wxListCtrl` doesn't delete its client data though, so from compatibility
point of view it would be a really bad idea to make `wxDataViewListCtrl`
behave differently. Notice that `wxTreeCtrl` does delete its associated
client data so it's not a problem there. Of course, the stupid thing is
that changing this now will result in leaks in the code already using
`wxDataViewTreeStore::SetItemData()`. But I think it's more important to
be compatible with 2.8 `wxListCtrl` as, again, this is the main purpose
for this entire class existence.
`GetItemCount()`: sure, thanks.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:12>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
`wxListCtrl` doesn't delete its client data though, so from compatibility
point of view it would be a really bad idea to make `wxDataViewListCtrl`
behave differently. Notice that `wxTreeCtrl` does delete its associated
client data so it's not a problem there. Of course, the stupid thing is
that changing this now will result in leaks in the code already using
`wxDataViewTreeStore::SetItemData()`. But I think it's more important to
be compatible with 2.8 `wxListCtrl` as, again, this is the main purpose
for this entire class existence.
`GetItemCount()`: sure, thanks.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:12>
wxTrac
May 25, 2012, 7:48:41 AM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:13>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Comment(by Kry):
Ok, so I'm attaching a new patch with the discussed changes.
I changed all wxClientData* to wxUIntPtr in the wxDataViewList* classes,
and made the store not delete the data on the destructor AND when
replacing the data, for consistency.
Added the GetItemCount as discussed, and documented all with @since 2.9.4
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:13>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Ok, so I'm attaching a new patch with the discussed changes.
I changed all wxClientData* to wxUIntPtr in the wxDataViewList* classes,
and made the store not delete the data on the destructor AND when
replacing the data, for consistency.
Added the GetItemCount as discussed, and documented all with @since 2.9.4
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:13>
wxTrac
May 25, 2012, 1:02:33 PM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:14>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Comment(by VZ):
(In [71561]) Use wxUIntPtr instead of wxClientData in wxDataViewListCtrl.
Do not delete the client data in wxDataViewListCtrl, this class mainly
exists
for compatibility with wxListCtrl and as the latter doesn't delete its
client
data, neither should the former.
See #11088.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:14>
#11088: Addition of wxDataViewListStore::Set/GetItemData
------------------------------------------------+---------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: confirmed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Keywords: wxDataViewListCtrl API docs-needed | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
(In [71561]) Use wxUIntPtr instead of wxClientData in wxDataViewListCtrl.
Do not delete the client data in wxDataViewListCtrl, this class mainly
exists
for compatibility with wxListCtrl and as the latter doesn't delete its
client
data, neither should the former.
See #11088.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:14>
wxTrac
May 25, 2012, 1:02:37 PM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:15>
#11088: Addition of wxDataViewListStore::Set/GetItemData
--------------------------+-------------------------------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: closed
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Changes (by VZ):
* status: confirmed => closed
* resolution: => fixed
Comment:
(In [71562]) Add wxDataViewListCtrl::{Set,Get}ItemData() methods.
These methods are convenient when migrating the code that previously used
wxListCtrl to wxDataViewCtrl.
Closes #11088.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:15>
#11088: Addition of wxDataViewListStore::Set/GetItemData
Reporter: lucak | Owner:
Type: enhancement | Status: closed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Resolution: fixed | Keywords: wxDataViewListCtrl API docs-needed
Component: GUI-all | Version: 2.9-svn
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Changes (by VZ):
* status: confirmed => closed
* resolution: => fixed
Comment:
(In [71562]) Add wxDataViewListCtrl::{Set,Get}ItemData() methods.
These methods are convenient when migrating the code that previously used
wxListCtrl to wxDataViewCtrl.
Closes #11088.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:15>
wxTrac
May 25, 2012, 1:02:42 PM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:16>
#11088: Addition of wxDataViewListStore::Set/GetItemData
--------------------------+-------------------------------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: closed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Resolution: fixed | Keywords: wxDataViewListCtrl API docs-needed
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
Comment(by VZ):
(In [71563]) Add wxDataViewListCtrl::GetItemCount().
This method is convenient and (almost, except for the return value)
compatible
with wxListCtrl.
See #11088.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:16>
#11088: Addition of wxDataViewListStore::Set/GetItemData
--------------------------+-------------------------------------------------
Reporter: lucak | Owner:
Type: enhancement | Status: closed
Priority: low | Milestone: 3.0
Component: GUI-all | Version: 2.9-svn
Resolution: fixed | Keywords: wxDataViewListCtrl API docs-needed
Blockedby: | Patch: 1
Blocking: |
--------------------------+-------------------------------------------------
(In [71563]) Add wxDataViewListCtrl::GetItemCount().
This method is convenient and (almost, except for the return value)
compatible
with wxListCtrl.
See #11088.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11088#comment:16>
Reply all
Reply to author
Forward
0 new messages