Bugzilla – Full Text Bug Listing
| Summary: | Grid needs clear methods | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Chuck Mastrandrea <Chuck.Mastrandrea> | ||||||||||||
| Component: | Nebula | Assignee: | Chris Gross <cgross> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||
| Severity: | normal | ||||||||||||||
| Priority: | P3 | CC: | tom.schindl | ||||||||||||
| Version: | unspecified | ||||||||||||||
| Target Milestone: | --- | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Windows XP | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
Created attachment 85651 [details]
Additions to GridItem.java (new clear method)
Hi Chuck, Good start. There are a couple of changes I think we need. First, clear-ing doesn't necessarily clear all the attributes. Specifically children, expanded state, level, visibility (this is really an internal state). Those shouldn't be cleared. Second, the methods in Grid should probably be more like the clear methods in Tree because the Grid is really a tree structure. This means we need a boolean argument for each method that tells us whether to also clear all the recursively. One other thing, when you attach things you should use the CVS patch format. You can create a patch in Eclipse really easily. Once you have a CVS project checked out and modified, just right-click on the root project tree element, go to Team - Create Patch. You will need to supply a filename but keep all the other options at the default. A patch file from that wizard will allow me to apply the patch using the Apply Patch wizard and using Eclipse's nice diff tools. Thats not a big deal with this kinda of additive only change but on other issues its a necessity. Thanks, -Chris Created attachment 86104 [details]
New clear methods in patch format
Thanks for the pointers Chris. I've made the recommended changes and am supplying them as a patch attachment.
Looking better. Still a few small issues I see. First - clearing should not change the hasChildren variable (since we aren't clearing the children). Also, can you actually remove the unnecessary code rather than just commenting it out. In the first clear method, there is no error checking. I know you are calling Grid.getItem which will throw the documented exceptions but in SWT the general rule is to fail as fast as possible. So can you add the checkWidget call and the invalid index check at the top of that method. Also, you don't really need the null checks after calling getItem or items.get(index). Those methods will either fail with an exception or return a valid item. I'd also just call the redraw() all the time. One redraw isn't very expensive so even if we are calling it when not necessary, its no big deal. If you remove logic around those two issues then it will help make the code a bit simpler (something I'm continually trying to do because the Grid is getting so large). Anyway, if you make those minor mods then I'm ready to commit. Thanks, -Chris Created attachment 86190 [details]
Updated patch with minor modifications
I've made the requested changes to the patch. Let me know if there are any other issues with this.
Hi Chuck, Can you try doing a CVS update on your grid project? I'm having trouble applying the patch and I think its because I've made changes to Grid and GridItem after you did your checkout. Could you do an update, get the latest code, and then recreate the patch file? Thx. Created attachment 86204 [details]
2nd try with new patch
Committed! Thanks for the contribution! |
Created attachment 85650 [details] Additions to Grid.java (the new clear methods) Currently the Grid component does not contain the clear methods for clearing one or more items in the receiver. Clearing resets various properties of items such as their text, icon and format settings. Following the same convention as the SWT Table, the following methods should be added to the Grid: clear(int index) clear(int start, int end) clear(int [] indices) clearAll() Chris noted in the eclipse.technology.nebula newsgroup that adding these methods would essentially complete the Grid API. The GridViewer could then be updated to support the "virtual" style of the Grid.