This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Provide all the API changes and let them review.
There is a document added, please have a look. At this moment it is a preliminary version. Some things are needed to consider. I.e. whether to make API symetric (with XML layer), or not, and let the definitions of groups (also modes possibly) just in layers?.... Or to be somewhere inbetween, and provide some kind of support also from API point of view?
Assigning to Pavel as a chairman of API Review board.
FYI - the review is on hold since the implementation team decided to run through an internal review first. When we start with review we will use the standard review process for this issue (http://openide.netbeans.org/tutorial/review-steps.html). Peter, when you are ready add link to answers for architecture questions and then reassign to me.
Assigning back to Pavel. The docs are prepared. Changes doc is available at: http://core.netbeans.org/windowsystem/changes.html (there are also added descriptions of persistence) The other docs I provided by other means (see the email).
Let's get started with review then. I will be on this review team and I am asking Tomas, Jarda and Jesse to participate. Please review the materials and add comments to this issue. If you find any TCRs/TCAs file bugs that this issue depends on. Let's hold the review meeting next Tuesday and I'll summarize results for 10/29.
Created attachment 11924 [details] zip containing javadoc and arch docs to new winsys
Hi I added the zip, it contains the javadocs and arch docs. Note: the changes in javadocs are marked XXX
Instructions how to build from the branch are at: http://core.netbeans.org/windowsystem/index.html or in the issue #29836 Note: Check whether you are using the up-to-date ones, they are going to be changed on per week basis.
Created attachment 11956 [details] Updated zip (see issue #36790)
IMO looks very good. Just some comments/questions. T01 There is no way to create a mode, the changes.html doc explains why it is not possible, but would be nice to give alternatives for former use cases (choosing another existing mode, using default editor mode, creating dialogs) etc. See also T05. T02 Could not mode creation still work in SDI? BTW what is the status of SDI mode? Is it still supported? Does not seem working wel... T03 Javadoc of Mode interface is not updated - it still advertises Workspace.createMode. T04 Javadoc of TopComponentGroup should mention it should never be implemented anywhere else than in the winsys code. T05 The use cases in changes document are not complete. There should be more use cases, also those already not supported (e.g. for floating windows) and should be more descriptive (e.g. say what is achieved via xml and what via programmatic API). Would be nice to have kind of migration guide for developers (I've found just some UI one on ui.netbeans.org, not referenced from the changes doc). T06 When will be openide/api/doc/org/openide/windows/doc-files/api.html updated? This is the doc that should contain complete description of the new state - consistent description is needed, not just changes. T07 Have concerns about functional (not API) compatibility. I've tried several things (old modules) - and at the best ended with things shown in the editor area separately without possibility to dock them somewhere else. Sometimes nothing appears at all, sometimes does not work (saw complaints by other people on non-adapted modules - e.g. java view, outline panel, task list). Is the "import" already implemented? Are there any useful test cases? Should not the user be allowed to arrange the "imported" components (i.e. dock them somewhere else than in the editor). T08 I've seen a fix in code commented like "Don't do any winsys manipulation from persistence routines!" Is this mentioned somewhere in the doc (not calling winsys from persistence code)? T09 Was there any performance testing/analysis done so far? For things like reading/storing the config, d&d, switching windows/groups, memory consumption etc)? T10 The PersistenceType property of TopComponents seems to but quite useful. Should it really be private? And should not be even provided by a real method in TopComponent class? All for now, Tomas
Mostly agree with Tomas' and Yarda's comments, except where noted. My own additions: $userdir/system/Windows2Local/WindowManager.wswmgr has no DOCTYPE specified. True in other XML files too, pls. correct. .wswmgr spelling errors: 'centered-horizontaly' and 'centered-verticaly' (please run a spellchecker on all DTDs!). "-ally" not "-aly" <maximized-mode name="" /> seems a clumsy way to specify that nothing is maximized. Ditto <active-tc id="" />. <other permanent="true" /> - "other"?? Weird XML element naming. "noNameMode_nnn" should probably be "anonymousMode_nnn". Don't understand distinction between Windows2 and Windows2Local folders. In my userdir (current winsys2 branch build, fresh user dir), *both* are created on disk (in $userdir/system/), contrary to the changes doc (3.4.2) which says that Windows2 is read-only. Windows2 for me contains only the Components subdir, which is not in Windows2Local. But the changes doc goes on to say (later in 3.4.2) that Components should only be in Windows2. So it contradicts itself. Please clarify. Use of element name <properties/> in tc-ref and group and tc-group DTDs is confusing to me. Properties of what? I agree with T10 & Y06 - almost everyone needs to use PersistenceType in practice, so it should probably be publicly committed to (not necessarily right now). In fact I find it very odd that the default is that top components are persisted even when closed. I rarely want something which is not visible to be stored on disk and consuming memory! You should need to ask to keep TC state around when the TC is closed, IMHO. Disagree with Y10 - getId has to be final to have the proper semantics, and if someone had such a method before, too bad (letting them override it now would just cause worse problems). And preferredId() *is* necessary, since the TC needs to state what it would like its ID to be, but the winsys impl needs to make the final decision based on the uniqueness constraint.
CC'ing Marek to comment he persistence questions. T01 Do you mean, to provide just usecases for finding the existing modes? Or provide new API for e.g. default editor mode? T02 Yes, this is a question for you, since in SDI the mode creation has sense comparing to the splits. Please decide. SDI was not described in UI spec, but it was tried to keep somehow possible. It should be working moreless fine now. T03, T04 I'll update that T05 The usecases could be added. The migration guide is moreless effective to XML layers (Marek). The old code will work in some way reasonable in the context of new layout.. I guess it is understandable that instead of floating mode (which is not supported) it is used some mode in the splits. T06 I didn't think about this.. so I have to do that T07 The import is implemented just a couple of days I guess. Here is necessary to provide the concrete cases.. and just then is possible to judge what is wrong. I believe, from new winsys point of view it should work fine. Please try out latest builds and fire issues. T08 I think this has to be understandable from general. Persistent routines should not do anything beside the serialization/deserialization work, I guess it could be find somewhere in jsdk serialization docs. To that mentioned case: it didn't break the new winsys, it was just an annoyance, and the code was needless in fact. T09 This is a question for other guys dealing with this, cc'ing Dafe and Radim. I don't know about concrete results yet. T10 Please decide. And also what should be the defeault
Response to Jesse's comments: >$userdir/system/Windows2Local/WindowManager.wswmgr has no DOCTYPE >specified. True in other XML files too, pls. correct. We decided not to save it to improve performance in old winsys. I will check it again if it has any impact. > .wswmgr spelling errors: 'centered-horizontaly' and > 'centered-verticaly' (please run a spellchecker on all DTDs!). > "-ally" not "-aly" I will fix that. > <maximized-mode name="" /> seems a clumsy way to specify that > nothing is maximized. Ditto <active-tc id="" />. Any better way? Should we add additional attribute to specify if attribute name/id is set? > <other permanent="true" /> - "other"?? Weird XML element naming. This element is for unspecific attributes. There was also "mode-state" visible/hidden before. Should we introduce new element for every different attribute? > "noNameMode_nnn" should probably be "anonymousMode_nnn". It is to Peter who creates mode name. > Don't understand distinction between Windows2 and Windows2Local > folders. In my userdir (current winsys2 branch build, fresh user > dir), > *both* are created on disk (in $userdir/system/), contrary to the > changes doc (3.4.2) which says that Windows2 is read-only. Windows2 > for me contains only the Components subdir, which is not in > Windows2Local. But the changes doc goes on to say (later in 3.4.2) > that Components should only be in Windows2. So it contradicts > itself. > Please clarify. Yes it should be described more precisely. Windows2, Windows2/Modes, Windows2/Groups are read only. Windows2/Components is not read only because serialization of TopComponents is handled by InstanceDataObject and separation of WindowManager, Modes, Groups,... was enough to solve problems with old nonseparated folders. > Use of element name <properties/> in tc-ref and group and tc-group > DTDs is confusing to me. Properties of what? Any better idea? I wanted to use one element for general info about tc-ref, group, tc-group but did not want to put them into root element where only version attribute is placed.
> "noNameMode_nnn" should probably be "anonymousMode_nnn". It is to Peter who creates mode name. I'll change it that way.
> T07 The import is implemented just a couple of days I guess. Here is > necessary to provide the concrete cases.. and just then is possible > to judge what is wrong. I believe, from new winsys point of view it > should work fine. Please try out latest builds and fire issues. Cannot try, see issue 36890.
Some comments to Peter's answers: > T01 Do you mean, to provide just usecases for finding the existing > modes? Or provide new API for e.g. default editor mode? I meant to explain functional alternatives instead of creating a new mode. I already know that one can reconsider the existing design and use e.g. editor mode instead, or decide to use another existing mode, or use dialog instead to keep the window floating. I think it should be mentioned somewhere that these are the alternatives for already not supported use case - creating a new mode. The fact that the API still can be called and does "something" is different thing. > T02 Yes, this is a question for you, since in SDI the mode creation > has sense comparing to the splits. Please decide. You should know all the reasons whether this should be done or not. I would say yes, but there can be another reasons against it (e.g. unsymmetrical funct. with MDI, SDI is edge case so this is not worth implementing, etc). > T05 The usecases could be added. The migration guide is moreless > effective to XML layers (Marek). The old code will work in some way > reasonable in the context of new layout.. I guess it is > understandable that instead of floating mode (which is not > supported) it is used some mode in the splits. Yes it is understandable how the old code (e.g. for modes and workspaces) will behave differently in the new winsys, but it is not clear how to rewrite the old code to new one to achieve the original functionality (e.g. instead of having the workspace you should define a window group now and reconsider the design). > T07 The import is implemented just a couple of days I guess. Here > is necessary to provide the concrete cases.. and just then is > possible to judge what is wrong. I believe, from new winsys point > of view it should work fine. Please try out latest builds and fire > issues. I'm just thinking if it would not be better not to pretend that everything is compatible and the old code will work "somehow" and rather say that old modules will need adjustments in most cases to work properly. It seems you don't have enough test cases to prove the functional compatibilty. The fact that it works "somehow" does not mean it is usable. > T09 This is a question for other guys dealing with this, cc'ing > Dafe and Radim. I don't know about concrete results yet. Would be nice to have at least some perf. numbers, not just say it looks fast enough...
here is link to Jarda's comments http://www.netbeans.org/servlets/ReadMsg?msgId=602334&listName=nbdev and response from Peter http://www.netbeans.org/servlets/ReadMsg?msgId=604702&listName=nbdev
Created attachment 12021 [details] Another update of WindowsAPI.zip
the decision of reviews is: Accepted with change requests (Go to implement or commit with completed Technical Change requests) There are 5 TCRs that need to be completed before integration. Number of TCAs was suggested. opinions and decision of reviewes are summarized in http://openide.netbeans.org/tutorial/reviews/opinions_36122.html
There are just two TCR's unresolved yet. Both belong to Dafe, so assigning this to him, when finished with them, assign to Pavel.
Two remaining TCR's are completed now, passing to Pavel.
so please close this issue as FIXED, Pavel & Co can verify and mark this issue as VERIFIED if they agree
Closing on Trung's order! :-)