Bug 198859 - multiple window layouts - API review
multiple window layouts - API review
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Window System
7.1
All All
: P2 (vote)
: 7.1
Assigned To: Stanislav Aubrecht
issues@platform
: API, API_REVIEW_FAST
Depends on:
Blocks: 198856
  Show dependency treegraph
 
Reported: 2011-05-24 15:46 UTC by Stanislav Aubrecht
Modified: 2011-06-16 13:54 UTC (History)
4 users (show)

See Also:
Issue Type: TASK
:


Attachments
proposed patch & api changes (18.53 KB, patch)
2011-05-24 15:47 UTC, Stanislav Aubrecht
Details | Diff
new patch (10.59 KB, patch)
2011-05-26 08:42 UTC, Stanislav Aubrecht
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stanislav Aubrecht 2011-05-24 15:46:27 UTC
the current implementation of netbeans window system has only one set of configuration files that define window layout - the defaults in Windows2 folder and user's modifications in <userdir>/Windows2Local.

this API changes adds support for multiple window layouts. each layout - role or perspective - will provide additional TopComponents, Modes, Groups and/or replace existing items in the default Windows2 layout. 

each role will be defined in "Window2/Roles/<role name>". the content of an individual role folder will have the same structure as the Windows2 folder. when the window system loads, it will merge the content of current role folder with the defaults from Windows2 folder and copy the merged contents to Windows2Local-<role name> folder.
note: if a role needs to 'hide' a file from Windows2 folder then it needs to define an empty file with the same name in its subfolder.

the following code then must be added to ModuleInstall.restored() method to switch window layout at startup:

WindowManager.getDefault().addWindowSystemListener( new WindowSystemListener() {

            @Override
            public void beforeLoad( WindowSystemConfiguration config ) {
                config.setRoleName( "roleName" );
            }

            @Override
            public void afterLoad( EventObject event ) {
            }

            @Override
            public void beforeSave( EventObject event ) {
            }

            @Override
            public void afterSave( EventObject event ) {
            }
        });
Comment 1 Stanislav Aubrecht 2011-05-24 15:47:40 UTC
Created attachment 108482 [details]
proposed patch & api changes
Comment 2 Jesse Glick 2011-05-24 16:35:44 UTC
By the way there is no clear reason why bug #198856 is separate from this. You do not need a separate issue just to file an API review.


[JG01] Using an empty file to mask an item with the same name is nonstandard; normally you would use *_hidden files. I would expect the behavior of a Roles/* folder to be like MultiFileSystem.

In fact I have already written a more generic mechanism that lets you dynamically merge in customizations to _any_ part of the system file system; http://blogs.oracle.com/geertjan/entry/jdesktoppane_on_the_netbeans_platform alludes to this.

I understand however that the window system is a little "special" in how it uses the SFS; at least, storing customizations in $userdir/config/Windows2Local/ rather than simply in $userdir/config/Windows2/ is inconsistent with all other kinds of SFS-based configuration in the platform. I am not sure about the historical reasons for this choice, nor whether the window system even listens to changes in the Windows2 folder.


[JG02] WindowSystemListener is using a nonstandard variant of the event listener pattern. All the methods, not just beforeLoad, should take WindowSystemConfiguration. And this should be renamed WindowSystemEvent. Anyway calling a setter on an event object is very unusual.

Or, if the only use case for WindowSystemListener is setting the role, then do this entirely differently:

class WindowManager {
  /** if during startup, will take effect before the window system loads */
  public void setRole(String roleName);
}

It is not even clear to me why you would do this from a ModuleInstall anyway; there could only be one module doing so! The typical use case would be switching to a different role in response to some user action, right? (For example, Eclipse's Debug perspective is opened when you debug a project.)


[JG03] What is mode-properties2_4.dtd for? There is no mention of this elsewhere in the issue or diff.

(It would be easier to see any changes from mode-properties2_3.dtd if you had used 'hg cp', by the way - then it would appear as a copy-plus-edit, rather than as a fresh file with no history. You also need to define diff.git=1 in your .hgrc.)


[JG04] Documentation about an API feature belongs in certain places - actual Javadoc comments where tied to a particular class or method, */src/**/doc-files/api.html otherwise. apichanges.xml should be used only to _mention_ that a change was made, not as the primary reference.
Comment 3 Stanislav Aubrecht 2011-05-25 10:25:11 UTC
JG01: window system does listen to changes in Windows2 folder. so when a module is enabled/disabled new windows may open/close at the right locations.

a role must be allowed to hide some files from the default window system layout. if we can use some file system trick to merge Windows2 folder with Windows2/Roles/SomeRole folder then it would be great. otherwise the window system would have to merge those folders 'manually' but it wouldn't see *_hidden files. hence the workaround with empty files.

JG02: the main purpose of WindowSystemListener is to enable better integration of NetBeans winsys into the next version of JDeveloper.

to switch to a new role the window system must first store current user settings under the current role, set new role name and then load window layout from the new role. so the best time to switch a role is just before window system loads.
if somebody wants to implement window layout/perspectives switcher, they need to register WindowSystemListener, show a dialog window listing available roles and then invoke 'Reload Windows' action. during the reload, the window system will ask for a new role using the registered listener.

JG03: the changes in mode DTD are actually unrelated to this API change. they're needed to implement some other window system features (minimize/undock/drag window group)

JG04: i'll update the documentation
Comment 4 Jesse Glick 2011-05-25 18:46:56 UTC
(In reply to comment #3)
> if we can use some file system trick to merge Windows2 folder with
> Windows2/Roles/SomeRole folder then it would be great.

Well, try using MultiFileSystem as this would handle merging automatically, including standard *_hidden masks.

> if somebody wants to implement window layout/perspectives switcher, they need
> to register WindowSystemListener, show a dialog window listing available roles
> and then invoke 'Reload Windows' action. during the reload, the window system
> will ask for a new role using the registered listener.

This seems very roundabout, and just confirms that the use cases do not imply the need for WindowSystemListener - a simple WindowManager.setRole(String) call would suffice, where the WM impl would know to do things in a certain order. You could call setRole during ModuleInstall.restored and the winsys would be loaded with that role active, though this seems like something no module should do - the winsys should rather simply persist the active role from the last shutdown; or a switcher dialog could call setRole at any time.

(If the _only_ use case for switching roles is from a switcher dialog, and not - say - in response to a domain-specific user gesture as in Eclipse, then it would be better not to make any API to switch roles at all; put the switcher dialog directly into core.windows.)

> JG03: the changes in mode DTD are actually unrelated to this API change.
> they're needed to implement some other window system features
> (minimize/undock/drag window group)

OK, so remove them from this diff.
Comment 5 Jaroslav Tulach 2011-05-25 19:12:43 UTC
(In reply to comment #4)
> Well, try using MultiFileSystem as this would handle merging automatically,
> including standard *_hidden masks.

We were thinking about this with Standa and there does not seem any way to reuse _hidden files for this purpose.

> This seems very roundabout, and just confirms that the use cases do not imply
> the need for WindowSystemListener - a simple WindowManager.setRole(String) call
> would suffice, 

The problem is the scope. setRole method is available all the time and people might believe they may call it (which is not what we want). The window system is still in the charge and only asks some other modules to advice it at a specific time what role to use. The callback on an "event" object of a listener exactly reflects this "call this method only now" requirement. Sure we might have:

public abstract class RoleNameProvider {
  protected abstract String selectedRoleName();
}

and find it through Lookup.getDefault()...
Comment 6 Stanislav Aubrecht 2011-05-26 08:19:57 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > if we can use some file system trick to merge Windows2 folder with
> > Windows2/Roles/SomeRole folder then it would be great.
> 
> Well, try using MultiFileSystem as this would handle merging automatically,
> including standard *_hidden masks.

can you pls provide a code snippet illustrating how to merge Windows2 folder with Windows2/Roles/SomeRole. the solution must also fire correct events when an enabled modules adds/removes some content in the Roles folder.

> 
> > if somebody wants to implement window layout/perspectives switcher, they need
> > to register WindowSystemListener, show a dialog window listing available roles
> > and then invoke 'Reload Windows' action. during the reload, the window system
> > will ask for a new role using the registered listener.
> 
> This seems very roundabout, and just confirms that the use cases do not imply
> the need for WindowSystemListener - a simple WindowManager.setRole(String) call
> would suffice, where the WM impl would know to do things in a certain order.
> You could call setRole during ModuleInstall.restored and the winsys would be
> loaded with that role active, though this seems like something no module should
> do - the winsys should rather simply persist the active role from the last
> shutdown; or a switcher dialog could call setRole at any time.
> 
> (If the _only_ use case for switching roles is from a switcher dialog, and not
> - say - in response to a domain-specific user gesture as in Eclipse, then it
> would be better not to make any API to switch roles at all; put the switcher
> dialog directly into core.windows.)

i don't plan to add any UI for role switching to NetBeans IDE - unless our HIE experts say it's a good idea.
but i want allow platform app developers to implement role switching and i can't find any strong argument against having method WindowManager.setRole(String)...
Comment 7 Stanislav Aubrecht 2011-05-26 08:42:51 UTC
Created attachment 108516 [details]
new patch
Comment 8 Jesse Glick 2011-05-26 14:24:07 UTC
(In reply to comment #5)
> setRole method is available all the time and people
> might believe they may call it (which is not what we want).

Apparently you wanted _someone_ to call it. So back to the question - what exactly are the use cases here?

> The window system
> is still in the charge and only asks some other modules to [advise] it at a
> specific time what role to use.

What is that specific time? The only code sample given so far is from ModuleInstall.restored, which seems clearly inappropriate.

(In reply to comment #6)
> can you pls provide a code snippet illustrating how to merge Windows2 folder
> with Windows2/Roles/SomeRole. the solution must also fire correct events when
> an enabled modules adds/removes some content in the Roles folder.

Code similar to that in the blog entry mentioned in JG01 should do this. That example used separate layer.xml fragments as the only layer provider; it is also possible to use a subdir of the SFS.

(I did not include that example since there is a bug in DataShadow - if shadow.primaryFile.fileSystem != Repository.default.defaultFileSystem then the interpretation of the originalFile attr is broken, which prevents @ActionReference etc. from working. But the winsys does not use shadows anyway.)

So use something along these lines (did not test this exact code yet):

final String role = ...;
FileSystem sfs = FileUtil.getConfigRoot().getFileSystem();
FileSystem roled = new MultiFileSystem(new FileSystem[] {sfs}) {
  public @Override FileObject findResource(String name) {
    return super.findResource(convert(name));
  }
  protected @Override FileObject findResourceOn(FileSystem fs, String res) {
    return super.findResourceOn(fs, convert(res));
  }
  String convert(String path) {
    if (path.startsWith("Windows2/")) {
      return "Windows2/Roles/" + role + path.substring("Windows2".length());
    } else {
      return path;
    }
  }
  public @Override void addNotify() {}
  public @Override void removeNotify() {}
};
FileSystem merged = new MultiFileSystem(new FileSystem[] {roled, sfs}) {
  public @Override void addNotify() {}
  public @Override void removeNotify() {}
};

> i don't plan to add any UI for role switching to NetBeans IDE [...]
> but i want allow platform app developers to implement role switching

If that is the only use case, then you need no API - just create the UI in core.windows, register an action to display it with @ActionRegistration but no @ActionReference, and leave it to an app to choose to display this action in e.g. the Windows menu.

> i can't find any strong argument against having method
> WindowManager.setRole(String)...

Then you probably do not need a listener; you just need to document in setRole that the visual effects of the role switch may be delayed a couple of seconds while the old user-customized layout is saved, new role defaults are loaded, and any new user-customized layout is loaded.
Comment 9 jrojcek 2011-05-26 15:08:55 UTC
Stando, if you Cc-ed me regarding the question about Role switching UI in NetBeans, then my answer is that we don't plan to have it in NetBeans IDE. Also my understanding is that JDev wants to permit role switching only during startup (first, or every time?), not anytime when JDev is running. I hope I'm not confusing roles with task/editor based layouts.
Comment 10 Jesse Glick 2011-05-26 15:24:53 UTC
(In reply to comment #9)
> permit role switching only during startup

This use case would be better served by a CLI option interpreted by core.windows:

  --role db-admin
Comment 11 Stanislav Aubrecht 2011-05-30 08:27:45 UTC
(In reply to comment #8)
> > i don't plan to add any UI for role switching to NetBeans IDE [...]
> > but i want allow platform app developers to implement role switching
> 
> If that is the only use case, then you need no API - just create the UI in
> core.windows, register an action to display it with @ActionRegistration but no
> @ActionReference, and leave it to an app to choose to display this action in
> e.g. the Windows menu.

there are many ways to implement role switching UI - modal dialog box, dropdown menu, combo box in a toolbar or even automatic switching when some other action is triggered in the platform app. i think it's best to provide API to platform developers and let them implement the UI themselves.

> 
> > i can't find any strong argument against having method
> > WindowManager.setRole(String)...
> 
> Then you probably do not need a listener; you just need to document in setRole
> that the visual effects of the role switch may be delayed a couple of seconds
> while the old user-customized layout is saved, new role defaults are loaded,
> and any new user-customized layout is loaded.

i still need the listener to integrate the NB window system into JDev. i could do it without a new public API using implementation dependency on core.window. but i think platform developers would benefit from the listener API as well.
Comment 12 Stanislav Aubrecht 2011-06-01 08:18:51 UTC
unless there are no more objections, i'll start integrating the new patch later on this week.
Comment 13 Jan Lahoda 2011-06-01 08:31:32 UTC
setRole/getRole do not have @since tag.
Comment 14 Stanislav Aubrecht 2011-06-01 08:38:12 UTC
(In reply to comment #13)
> setRole/getRole do not have @since tag.

i'll fix that
Comment 15 Jesse Glick 2011-06-01 19:32:33 UTC
Details of the semantics of roles (folder locations, masking, etc.) should be in WindowManager.setRole, not in apichanges.xml.

apichanges.xml should use <class .../> on WindowManager, WindowSystemListener, and WindowSystemEvent.

Also, <issue number="198856"/> should be <issue number="198859"/>.

And do not forget to update <date>.

ModuleInstall.restore - should be "restored".

BTW do not use the IDE's built-in diff for creating patches for review. Configure diff.git=1 and use 'hg di' (or MQ, etc.).
Comment 16 Stanislav Aubrecht 2011-06-06 14:36:20 UTC
API changes are in 28e12812f968

the actual implementation will follow later
Comment 17 Stanislav Aubrecht 2011-06-06 15:36:17 UTC
WindowSystemListener impl: 51065bf0a248
Comment 18 Jesse Glick 2011-06-08 15:24:44 UTC
[JG05] Consider following http://wiki.netbeans.org/CompatibilityPolicy#Technical_procedure_for_making_an_incompatible_phased_change to remove org.openide.windows.Workspace (and related methods), which is effectively superseded by this new API.

There might be other deprecated stuff in o.o.windows that is truly unused and could be removed at the same time (but TopComponent.requestFocusInWindow probably needs to be undeprecated).
Comment 19 Quality Engineering 2011-06-10 14:48:02 UTC
Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/28e12812f968
User: S. Aubrecht <saubrecht@netbeans.org>
Log: #198859 - API changes to support multiple window layouts
Comment 20 Geertjan Wielenga 2011-06-16 13:19:59 UTC
I am missing a "getRoles" method, i.e., this method will provide a list of all the roles defined in the system, not just the current role, which is what "getRole" does. I think "getRoles" is essential (e.g., to display in a GUI component so that the user can choose a role). Hence, reopening.
Comment 21 Stanislav Aubrecht 2011-06-16 13:54:12 UTC
getRoles() == FileUtil.getConfigFile("Windows2/Roles").getChildren()

also note you can switch to a role which does not have a subfolder under Windows2/Roles. in that case the initial window layout will be the same as the default one but user's changes will be persisted to a different location.


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