Bug 46167 - Need access to tree node expansion
Need access to tree node expansion
Status: VERIFIED FIXED
Product: debugger
Classification: Unclassified
Component: Code
4.x
Sun All
: P2 (vote)
: 4.x
Assigned To: apireviews
issues@debugger
: API, API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-07-14 23:17 UTC by ivan
Modified: 2005-04-15 14:54 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
Diff attached (94.02 KB, patch)
2005-01-13 13:08 UTC, Jan Jancura
Details | Diff
Diff updated (102.59 KB, patch)
2005-01-18 10:33 UTC, Jan Jancura
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ivan 2004-07-14 23:17:39 UTC
We have a scheme where we remember which variables
were expanded by the user (pointer-chased) so
the next time we get to the same context/function
the user doesn't have to re-expand them.

This functionality depends on our ability to:
- detect that a node was expanded or closed.
  The expansion we can detect for free usually
  by tracking state and seeing getChildren() 
requests.
  The closing is trickier and in the past we had to 
  dig deep and get a
TreeTableExplorerViewSupport.MyTreeTable in order
to be able to listen to
TreeExpansionListener.treeCollapsed().

- expand or collapse a node programmatically:
   ((TreeView)tview.myTreeTable).expandNode(node);

- ask whether a node is expanded or collapsed.

So some enhancement to spi.modelview.TreeModel
seems to be in order.
Comment 1 Chihin Ko 2005-01-05 01:19:38 UTC
Hanz,

  How is going with this API request ? I'm working on Local/Watch views
of VENUS, this is the only work left, can I have it ASAP ?
Comment 2 Jan Jancura 2005-01-06 17:30:59 UTC
Unfortunatelly I am solving several high importance ASAP problems now.
This one is not easy (API change). I hope that I will have a chance to
fix it for NB4.1, but I am not 100% sure.
Comment 3 Chihin Ko 2005-01-07 23:33:46 UTC
Hanz,

  It's OK to put it in NB 4.1, we'll use 4.1 for VENUS sunstudio.
If you can get it done before end of Jan, I'll appreciate it.
Comment 4 Jan Jancura 2005-01-13 13:07:33 UTC
This bug requires API change.
Comment 5 Jan Jancura 2005-01-13 13:08:53 UTC
Created attachment 19667 [details]
Diff attached
Comment 6 Jan Jancura 2005-01-13 13:44:09 UTC
This change has been done to cover user requirement #46167.
List of changes:
    1) new TreeExpansionModel introduced
    2) new class Models.TreeFeatures contains tree expansion methods.
During implementation of this user requirement I have discovered
important problem in ViewModel API. 
There is no possibility to add a new model to it. List of changes
fixing this problem:
    1) Models.createView (...) and Models.setModelsToView (...)
methods do not use fixed set of models as parameters,
       but they use Models.CompoundModel.
    2) Models.CompoundModel is final class implementing all currently
supported models. The only way how to create
       a new instance of it is Models.createCompoundModel (List
listOfModels). It allows future additions of new
       models.
    3) Methods Models.createCompoundTreeModel (...),
Models.createCompoundNodeModel (...), 
       Models.createCompoundTableModel (...),
Models.createCompoundNodeActionsProvider (...), and field
       Models.EMPTY_TREE_MODEL has been removed as useless.
How to update your code using these exceptions:
    No changes required for most of clients. You should change your
code only if you are providing some 
    new view based on ViewModel. In this case you should put all your
model instances to one list, 
    and call Models.createCompountModel (list), in place of calling
create*Model* methods.
Comment 7 Jaroslav Tulach 2005-01-14 08:13:06 UTC
1. the diff is not final, missing javadocs, @since tags, apichanges,
specver + impl version increment, etc.

2. there are no tests and there should be. minimal coverage shall
cover invokeLater calls and ability to change the model, which was
described as one of the major achievements

3. I'd like to question the need for incompatible changes. They do not
seem necessary for treeexpansion state at all, so this issue seems to
join more unrelated commits into one

4. I could not find interface Model. Who knows what it is for and why
it is not included in the diff nor in current state of the api

5. I suggest to turn this into real review. The incompatible changes
in debugger are larger and larger and I have no trust that there is
ever going to be a day when the team will be able to develop compatibly. 

The first available day for review is Tuesday, if Hanz thinks it is
suitable date for him.
Comment 8 Jan Jancura 2005-01-14 14:54:50 UTC
add 1) diff is not final, because review is in progress.
JavaDoc - you are right, Javadoc for one method is missing. I will fix
it. Not a problem.
apichanges - I have copied text from apichanges document to this
issue, do you need somethink more?
version numbers - I plan to change major version number after all
changes planned for 4.1.

add 2) Sorry, but I do not understand: "cover invokeLater calls and
ability to change the model". 

add 3) I think that incompatible change is needed in this case.

add 4) Model interface in marker interface with no methods. Its not in
diff because I have forgot to add it to cvs before diff. 

add 5) Developing compatibly is not problem - ist possible and not
"hard". But its not our primary goal now. Our plan is to incorporate
feedback and additional requirements from downstream teams to our API,
and finalize it (see NetBeans promo E PCD).

I am not against regular review and Tuesday is OK for me. But do you
think that you will have some realy technical comments? Formal things
like JavaDoc & tests do not need special meeting, I think.


Comment 9 Jaroslav Tulach 2005-01-18 10:11:24 UTC
This does not seem to be non-contraversal. Let's dicuss this today.

Re.Re.1 - would be nice to see the final diff at least a day before
integration

Re.Re.2 - I've noticed usage of invokeLater. It (hopefully) has some
meaning. I want the meaning to be documented by a test.

Re.Re.3 - I do not understand why incompatible change is needed. We'll
discuss that.

Re.Re.5 - This is not what you promised in
http://openide.netbeans.org/tutorial/reviews/opinions_27194.html
it says stable for promo D. If you wished to finalize it later, you
would have to ask for permition to use /0 in its name, which you did
not. See http://openide.netbeans.org/tutorial/api.html#restrictions
Comment 10 Jan Jancura 2005-01-18 10:33:30 UTC
Created attachment 19740 [details]
Diff updated
Comment 11 Jaroslav Tulach 2005-01-18 12:53:57 UTC
6. apichanges.xml is not in html and would be misformated.
Comment 12 Jaroslav Tulach 2005-01-18 21:54:14 UTC
After discussion with Hanz today we agreed that it is ok, to do this
incompatible change (and the change in issue 53073 and one more which
will change the signature of TreeModelListener to include one method
with TreeModelEvent) if:
1. all changes will be notified on nbdev@
2. all changes will be done for 4.1
3. tests will be provided as requested above and in other issues
4. under the promise that "Hanz and nobody else will not incompatibly
change the api after 4.1"
Then the integration can proceed.
Comment 13 Jan Jancura 2005-02-03 08:43:36 UTC
Code in trunk, review done, waiting on test -> no41
Comment 14 Jaroslav Tulach 2005-02-03 09:17:34 UTC
Waiver on test rejected. You new that tests are necessary condition
before integration. Without them the feature is not ready. It is your
reponsibility to fully implement the API change by covering it with
tests for 4.1.
Comment 15 Jaroslav Tulach 2005-02-10 19:25:18 UTC
Waiver on test rejected, even if we do not want to block beta with
this issue. We are ready to block release.
Comment 16 Chihin Ko 2005-03-04 00:22:43 UTC
I tried new API TreeExpansionModel, in general it works for most
cases, but when switching between other views and local/watch views,
the closed node in local view will be reopen, this is a bug from
debuggercore, here is the stack trace for it.

in setModel(), why does it do the expandNodes() without checking the
node expanding state ?

[t@23 l@5]: where
  [1]
com.sun.tools.debugger.dbxgui.debugger.DbxVariable.setExpanded(this =
<OBJECT>, root = "foob", e = true) 
"com/sun/tools/debugger/dbxgui/debugger/DbxVariable.java":353
  [2]
com.sun.tools.debugger.dbxgui.debugger.VariableModel.nodeExpanded(this
= <OBJECT>, node = <OBJECT>) 
"com/sun/tools/debugger/dbxgui/debugger/VariableModel.java":174
  [3]
org.netbeans.spi.viewmodel.Models.DelegatingTreeExpansionModel.nodeExpanded(this
= <OBJECT>, node = <OBJECT>) 
"org/netbeans/spi/viewmodel/Models.java":1312
  [4]
org.netbeans.spi.viewmodel.Models.CompoundModel.nodeExpanded(this =
<OBJECT>, node = <OBJECT>)  "org/netbeans/spi/viewmodel/Models.java":2194
  [5] org.netbeans.modules.viewmodel.TreeTable.treeExpanded(this =
<OBJECT>, event = <OBJECT>) 
"org/netbeans/modules/viewmodel/TreeTable.java":184
  [6] javax.swing.JTree.fireTreeExpanded(--- Not compiled -g ---) 
"javax/swing/JTree.java":2238
  [7] javax.swing.JTree.setExpandedState(--- Not compiled -g ---) 
"javax/swing/JTree.java":2999
  [8] javax.swing.JTree.expandPath(--- Not compiled -g ---) 
"javax/swing/JTree.java":1735
  [9] org.openide.explorer.view.BeanTreeView.showPath(this = <OBJECT>,
path = <OBJECT>)  "org/openide/explorer/view/BeanTreeView.java":100
  [10]
org.netbeans.modules.viewmodel.TreeTable.MyTreeTable.expandNodes(this
= <OBJECT>, exPaths = <OBJECT>) 
"org/netbeans/modules/viewmodel/TreeTable.java":361
=>[11] org.netbeans.modules.viewmodel.TreeTable.setModel(this =
<OBJECT>, model = <OBJECT>) 
"org/netbeans/modules/viewmodel/TreeTable.java":135
  [12] org.netbeans.spi.viewmodel.Models.1.run(this = <OBJECT>) 
"org/netbeans/spi/viewmodel/Models.java":114
  [13] java.awt.event.InvocationEvent.dispatch(--- Not compiled -g
---)  "java/awt/event/InvocationEvent.java":178
  [14] java.awt.EventQueue.dispatchEvent(--- Not compiled -g ---) 
"java/awt/EventQueue.java":454
  [15] java.awt.EventDispatchThread.pumpOneEventForHierarchy(--- Not
compiled -g ---)  "java/awt/EventDispatchThread.java":201
  [16] java.awt.EventDispatchThread.pumpEventsForHierarchy(--- Not
compiled -g ---)  "java/awt/EventDispatchThread.java":151
  [17] java.awt.EventDispatchThread.pumpEvents(--- Not compiled -g
---)  "java/awt/EventDispatchThread.java":145
  [18] java.awt.EventDispatchThread.pumpEvents(--- Not compiled -g
---)  "java/awt/EventDispatchThread.java":137
  [19] java.awt.EventDispatchThread.run(--- Not compiled -g ---) 
"java/awt/EventDispatchThread.java":100
[t@23 l@5]: 
Comment 17 Jan Jancura 2005-04-15 14:24:03 UTC
Test is done, see:
debuggercore\viewmodel\test\unit\src\org\netbeans\api\viewmodel\BasicTest.java 

It opens some nodes, and checks if this event is notified.
Comment 18 Jaroslav Tulach 2005-04-15 14:54:56 UTC
Looks ok to me. 


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo