Community
Participate
Working Groups
As soon as a module-info.java file is created com.sun.* classes (e.g. com.sun.awt.AWTUtilities) as cannot be resolved any longer. Import error: "The import com.sun cannot be resolved"
AWTUtilities is in the java.desktop module which does not export com.sun.awt package, so this is by design For any other classes, make sure the package in which the said class resides is exported by the declaring module and your module reads the declaring module and if it still doesn't work, then it is a bug indeed. For example, if AWTUtilities is declared in java.desktop module, then the module-info in java.awt module should have the directive exports com.sun.awt; and your module-info should have the directive requires java.desktop; Please reopen if you find such a case where it doesn't work
I'm aware that AWTUtilities is not exported by java.desktop. However, we have to access these classes and users have to add "-add-exports". Javac also supports "--add-exports" to make these things possible, e.g. --add-exports=java.desktop/com.sun.awt=ALL-UNNAMED However, how can this issue be solved in Eclipse?
This will need some work in the core too. Perhaps Noopur can tell us what kind of API they need.
Perhaps JDT/UI will attach some extraAttribute to affected classpath entries, which JDT/Core than should translate into compiler options? S.t. like <attributes> <attribute name="addExports" value="java.desktop/com.sun.awt=ALL-UNNAMED"/> </attributes> Module name may be redundant for single-module artifacts, but is necessary for JRT, so most consistent, if always used. We could validate if the given module is contained in the corresponding classpath entry. That would also hint at a possible UI: an additional node in Java Build Path > Libraries and/or the "Edit JRE" dialog from JDT/Debug. OK?
New Gerrit change created: https://git.eclipse.org/r/102490
(In reply to Eclipse Genie from comment #5) > New Gerrit change created: https://git.eclipse.org/r/102490 This change adds support for two classpath attributes to JDT/Core: add-reads & add-exports. The values of these attributes exactly follow the syntax of the corresponding CLI options. So far no validation is included.
I also started on the UI side. As a first attempt, I made the build-path > library page simply display all add-exports as direct children of their classpath entry, but this would mean unbounded growth of that list of children if there are many add-exports. Thus I propose to combine "module" and "add-exports" nodes in the UI like this: - library children contain one "M" node like it is now, with label one of: - "Is not a module" (formerly: "false") - "Is a module" (formerly: "true") - "Is a module with 1 export added" (-"-) - "Is a module with /n/ exports added" (-"-) - then, when module has add-exports those are displayed as children of the "M" - image is a package icon with "export" decoration - label is like: "Add-exports: mod.one/my.pack=your.mod" - individual add-exports can be removed from the sub-list, or edited ("Edit") - for editing add-exports a dialog appears with three fields: - Source module ______________ - Package ______________ - Target modules (comma separated) ______________ I'm not 100% sure how to best support adding new add-exports. My current thinking is to combine this with "Toggle" into one "Edit" action on the "M" node, pulling up a dialog with these details: Checkbox "Contains one or more modules" Table of added exports with an [Add] button - this table is disabled if checkbox is unchecked. The alternative to add a button [Add export] in the library tab itself seems undesirable because (a) the list of buttons will get quite long and (b) that button would only be enabled if "M" is set to true. Thoughts? PS: what about JRT? This node by definition contains modules, not just one. Adding exports to this node is an important use case, so should "M" just say: "Contains modules" (with no option to turn this off)?
New Gerrit change created: https://git.eclipse.org/r/102544
(In reply to Eclipse Genie from comment #8) > New Gerrit change created: https://git.eclipse.org/r/102544 Unfinished draft for the UI part of this, so that people can play with it and comment. Needs latest from the Core part (https://git.eclipse.org/r/102490) to compile and run. Not yet: validation, editing assist etc.
Gerrit change https://git.eclipse.org/r/102490 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=2382903d523e8107d8ced048a61b925ed1fbb620
New Gerrit change created: https://git.eclipse.org/r/102568
(In reply to Stephan Herrmann from comment #9) > (In reply to Eclipse Genie from comment #8) > > New Gerrit change created: https://git.eclipse.org/r/102544 > > Unfinished draft for the UI part of this, so that people can play with it > and comment. > Needs latest from the Core part (https://git.eclipse.org/r/102490) to > compile and run. > > Not yet: validation, editing assist etc. In patch set #2 ( https://git.eclipse.org/r/#/c/102544/2 ) I extended the UI part to apply also to project dependencies. To avoid code duplication some code has been pulled up to BuildPathBasePage.
Gerrit change https://git.eclipse.org/r/102568 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=8af65b1cffd9d89bcc44847f7f1f3b58a5b432e5
(In reply to Eclipse Genie from comment #13) > Gerrit change https://git.eclipse.org/r/102568 was merged to [BETA_JAVA9]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=8af65b1cffd9d89bcc44847f7f1f3b58a5b432e5 > Improved the Core part: Support the new classpath attributes also in reconcile scenarios (SearchableNameEnvironment).
(In reply to Stephan Herrmann from comment #7) > I also started on the UI side. > > As a first attempt, I made the build-path > library page simply display all > add-exports as direct children of their classpath entry, but this would mean > unbounded growth of that list of children if there are many add-exports. > @Stephan, Sorry, I did not understand, what user should do if they want to use "add-exports" for a module. I tried the patch https://git.eclipse.org/r/#/c/102544/2 and got this exception when I click on "Edit", after selecting "Is not a Module". java.lang.ClassCastException: java.lang.String cannot be cast to [Lorg.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleAddExport; at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleDialog.createListContents(ModuleDialog.java:133) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleDialog.<init>(ModuleDialog.java:102) at org.eclipse.jdt.internal.ui.wizards.buildpaths.BuildPathBasePage.showAddExportDialog(BuildPathBasePage.java:64) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.editAttributeEntry(LibrariesWorkbookPage.java:521) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.editEntry(LibrariesWorkbookPage.java:477) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.libaryPageCustomButtonPressed(LibrariesWorkbookPage.java:269) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.access$0(LibrariesWorkbookPage.java:247) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage$LibrariesAdapter.customButtonPressed(LibrariesWorkbookPage.java:204) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField.buttonPressed(TreeListDialogField.java:171) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField.doButtonSelected(TreeListDialogField.java:402) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField.access$2(TreeListDialogField.java:398) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField$2.widgetSelected(TreeListDialogField.java:363) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:249) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:86) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4428) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4238) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3817) at org.eclipse.jface.window.Window.runEventLoop(Window.java:818) at org.eclipse.jface.window.Window.open(Window.java:794) at org.eclipse.ui.dialogs.PropertyDialogAction.run(PropertyDialogAction.java:157) at org.eclipse.jface.action.Action.runWithEvent(Action.java:473) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:565) at org.eclipse.jface.action.ActionContributionItem.lambda$4(ActionContributionItem.java:397) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:86) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4428) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4238) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3817) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1150) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1039) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:680) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:594) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:148) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:151) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:673) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:610) at org.eclipse.equinox.launcher.Main.run(Main.java:1519) at org.eclipse.equinox.launcher.Main.main(Main.java:1492)
Thanks, Sarika, for trying, (In reply to Sarika Sinha from comment #15) > Sorry, I did not understand, what user should do if they want to use > "add-exports" for a module. > I tried the patch https://git.eclipse.org/r/#/c/102544/2 > and got this exception when I click on "Edit", after selecting "Is not a > Module". This is correct. > java.lang.ClassCastException: java.lang.String cannot be cast to > [Lorg.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleAddExport; That's bad. As per my patch the intention is, that each CPListElementAttribute with key = "module" contains an array of ModuleAddExports (possible empty) or null. Browsing the code, I found one location that violates this assumption. Did you just add a jar file to the build path immediately before clicking "Edit"?
(In reply to Stephan Herrmann from comment #16) > Browsing the code, I found one location that violates this assumption. Did > you just add a jar file to the build path immediately before clicking "Edit"? This particular issue should be fixed in patch set #3
If we agree on the parts in Core & UI, leveraging the same for launching should be easy, since the value of this classpath attribute follows exactly the syntax from --add-exports according to JEP 261. (currently focusing on --add-exports, but at the end of the day, --add-reads should of course follow suite). @Sarika, do you want a separate bug for the launch story? Mhh, --add-opens is not relevant at build time, but may be needed at launch time. Do you have an idea how this could be consistently integrated?
(In reply to Stephan Herrmann from comment #17) > (In reply to Stephan Herrmann from comment #16) > > Browsing the code, I found one location that violates this assumption. Did > > you just add a jar file to the build path immediately before clicking "Edit"? > > This particular issue should be fixed in patch set #3 Thanks, Yes it is fixed now. Will go through the JDT UI changes in detail. Apart from exports we have following for testing and debugging but available in both phases, Compile and Launch - --limit-modules <module>(,<module>)* --add-reads <source-module>=<target-module> --patch-module <module>=<file>(<pathsep><file>)* Luanching will also need a JDT core API for --add-modules <module>(,<module>)* Have Created Bug 520601 to handle Launching areas to be fixed. Launching needs to support the segregation of Module Path and Class Path. For Launching, even without additional UI support, user will be able to add all the arguments even now, so based on priorities we will see what best can be provided for September release.
Combining this with the module attribute doesn't appeal to me - the module attribute allows the user to specify whether a particular entry goes on the module path or not and is not related to the additional exports/reads etc. An additional node under the module node or a separate tab for these additional options seems more appropriate to me. The edit dialog for add-exports can remain the same then, with three fields and we will find the appropriate entry and add the attribute to it. In addition, adding exports should be possible from the individual entries as well, and there the edit dialog should already be populated with the source module (which is the name of the module corresponding to the selected entry) and the package names could be listed from the module (nice to have :-) ). Looking at all the options possible, all of these do not belong on the build path dialog because not all deal with the build path, several are just additional options. We probably should create a new build options dialog which contains the build path entries as well as additional tabs for options. That would give us the freedom to create as many views of our model as we like (grouped by entry kind, grouped by location kind, plain view without any grouping etc). We could also provide options for modifying the module graph there (limit-modules, add-modules).
(In reply to Sasikanth Bharadwaj from comment #20) > Combining this with the module attribute doesn't appeal to me - the module > attribute allows the user to specify whether a particular entry goes on the > module path or not and is not related to the additional exports/reads etc. Note, that this combination does not happen at the IClasspathAttribute, only in the visualization via CPListElement. Logically, both attributes are actually related, in that module=true is a precondition to defining add-exports. > An additional node under the module node or a separate tab for these > additional options seems more appropriate to me. Mmhh, each add-exports node currently *is* an additional node under the module node. Are you proposing a generic "add-exports" node that only serves as a parent for the individual add-exportss (similar to the "Access Rules" parent node, e.g.)? Lets step back and separate two groups of users: (1): Those who know about classpath attributes. For them "module" and "add-exports" are distinct things (no good alternative anyway, because attributes a not structured, each only has one simple value). (2): Those who only use the UI. For them, a library / project may or may not be a module. If it is, it may have additional module-related properties. Selecting the module node and clicking "Edit" means to edit the module-ness of this library - including some details. Implementation note: I found it difficult enough to make add-exports a child of Module. If we'd add one more level in the hierarchy, I might need some help how to implement this. (In reply to Sasikanth Bharadwaj from comment #20) > In addition, adding exports > should be possible from the individual entries as well, You mean the modules inside a JRT node? > and there the edit > dialog should already be populated with the source module (which is the name > of the module corresponding to the selected entry) and the package names > could be listed from the module (nice to have :-) ). Sure, that's what I meant with "editing assist", which is still missing. > Looking at all the options possible, all of these do not belong on the build > path dialog because not all deal with the build path, several are just > additional options. Add-exports and add-reads are part of the declaration of dependencies. I thought that's what the build path tabs are about. > We probably should create a new build options dialog how would that dialog relate to the properties dialog? > which contains the build path entries as well as additional tabs for > options. That would give us the freedom to create as many views of our model > as we like (grouped by entry kind, grouped by location kind, plain view > without any grouping etc). We could also provide options for modifying the > module graph there (limit-modules, add-modules). I think add-modules is done by adding more entries to the build paths. limit-modules can be associated to a specific build path entry. Note, that I'm a bit defensive because I want to provide something easily feasible NOW, that only slightly extends the existing concepts. If later we feel so, we may indeed provide UI that completely departs from the concept of a build path and make it a Module Graph UI. But I see time running out for Java 9 GA regarding any really fancy new UI.
@Stephan, Want to understand 2 things- 1) Is there any significance of having two different status ? "Is a module with 1 export added" (-"-) "Is a module with /n/ exports added" (-"-) 2) With current approach, How will user define these properties for com.sun.* or any module provided by JRE ? How about having only Is Module property with an overlay of "M"( or putting under Module Path Node) and one button "Module Properties" on Libraries Tab for all modules. Here user can define all options like --add-exports, --add-reads, etc for any module which is resolved. And this can be enhanced from basic text box entries to auto completion or group by several options. Advantage of this will be user need not search the Projects/Libraries and then provide the properties for it. At one place user can see over all what are the properties defined. User should be able to provide these options for user defined containers.
(In reply to Stephan Herrmann from comment #18) > > Mhh, --add-opens is not relevant at build time, but may be needed at launch > time. Do you have an idea how this could be consistently integrated? I don't see --add-opens described in any JEP ? I could only find it described in https://docs.oracle.com/javase/9/migrate/toc.htm#JSMIG-GUID-12F945EB-71D6-46AF-8C3D-D354FD0B1781 The syntax for --add-opens is: --add-opens module/package=target-module(,target-module)*
(In reply to Sarika Sinha from comment #22) > @Stephan, > Want to understand 2 things- > 1) Is there any significance of having two different status ? > "Is a module with 1 export added" (-"-) > "Is a module with /n/ exports added" (-"-) Just for correct grammar of singular vs. plural :) > 2) With current approach, How will user define these properties for > com.sun.* or any module provided by JRE ? In comment 7 I discussed adding the M also to the JRE / JRT node. In fact, that's the use case were specifying the Source module is relevant, in other cases this can be determined from the entry (and should be prefilled in the dialog). > > How about having only Is Module property with an overlay of "M"( or putting > under Module Path Node) and one button "Module Properties" on Libraries Tab > for all modules. This might be closer to the structure of the CLI option, but I feel the IDE can do better: show the user which module is affected by which option. In a perfect world, the module itself would declare the export. When you need add-export, you impose an export on a module, so doing it right at the module seems most appropriate to me. A far fetched comparison: consider a world where classes had no visibility modifier, and on the project properties you define a property that tells which classes are public ;p Re envisioned editing assist: - single module entry: - Source module: prefilled & unmodifiable - Package: content assist offers any non-exported package of the module - Target module(s): propose known modules (not all may be known in the WS) - multi module entry / container: - Source module: content assist offers any module in this container - etc see above One more use-case: removing a classpath entry. => Should automatically remove all --add-* that refer to this entry's module as the Source module. => If --add-* is a property of the entry itself, this feature is a no-op :) (If all options are collected in one place, this requires some magic) Also the analogy to Access rules points towards making these per-entry properties. It's almost the same story. The one open issue IMHO is: attach --add-* to the JRE node at top level, or to each individual module inside. Oops, on the Libraries tab, JRE still expands to jrt-fs.jar instead of listing the contain modules. This looks unfortunate to me. Anyway, when pretending this would list the individual modules, I could understand that searching the exporting module in this list could be felt as an unnecessary effort, but then: you do need to know the Source module any way. I have no strict opinion regarding JRE, though. > Advantage of this will be user need not search the Projects/Libraries and > then provide the properties for it. At one place user can see over all what > are the properties defined. Conceptually its a matrix of module x options. You seem to prefer to make a section for options, whereas I see the modules as the major grouping factor, where everything should be sorted under. One more idea: When you think about compatibility with javac & java, wouldn't it be cool if Build path and launch's Classpath had a button for "previewing" a suitable commandline for javac / java? Many times I run a program, and then need to go to the debug view and see which node has the command line in its properties just to figure out the bare command line. Having a shortcut for this would be awesome, if you ask me. For the issue at hand: such preview feature would allow us, to define the options in any structure we like and still enable the user to use the configured options for command line usage. > User should be able to provide these options for user defined containers. sure Last question: do you see any option that does not relate to any module in a classpath entry?
(In reply to comment #24) > (In reply to Sarika Sinha from comment #22) ... > > How about having only Is Module property with an overlay of "M"( or putting > > under Module Path Node) and one button "Module Properties" on Libraries Tab > > for all modules. > > This might be closer to the structure of the CLI option, but I feel the IDE can > do better: show the user which module is affected by which option. No doubt, the user must know which option affects which module > In a perfect world, the module itself would declare the export. When you need > add-export, you impose an export on a module, so doing it right at the module > seems most appropriate to me. > Agreed, the option has to be shown at the correct entry, and it is most suitable when the user has added a new entry to the build path and wants to add these options immediately. What I'm troubled by as a user is if I want to add this option later on, I need to find the entry that corresponds to the source module name I know, and then only I can add the option, where as the IDE could be more helpful by allowing me to type the source module name and find the corresponding entry itself. > A far fetched comparison: consider a world where classes had no visibility > modifier, and on the project properties you define a property that tells which > classes are public ;p > > Re envisioned editing assist: > - single module entry: > - Source module: prefilled & unmodifiable > - Package: content assist offers any non-exported package of the module > - Target module(s): propose known modules (not all may be known in the WS) Sounds good. I guess it's enough to propose all known modules on the module path because only those can be resolved anyway. Haven't checked what happens if the target module doesn't exist > - multi module entry / container: > - Source module: content assist offers any module in this container > - etc see above > Sounds good. It would be nice if the same is available in general as well, i.e. content assist offers any module on the module path. Others are the same as above. Just saves the effort of having to look for the specific entry > One more use-case: removing a classpath entry. > => Should automatically remove all --add-* that refer to this entry's module as > the Source module. > => If --add-* is a property of the entry itself, this feature is a no-op :) > (If all options are collected in one place, this requires some magic) > > Also the analogy to Access rules points towards making these per-entry > properties. It's almost the same story. > Counter argument - removing an add-* entry - again, finding the entry and then the option seems painful. There's no doubt that the property belongs to the entry. It's just the presentation and the ease of editing that we need to think about > > The one open issue IMHO is: attach --add-* to the JRE node at top level, or to > each individual module inside. Oops, on the Libraries tab, JRE still expands to > jrt-fs.jar instead of listing the contain modules. This looks unfortunate to me. > Anyway, when pretending this would list the individual modules, I could > understand that searching the exporting module in this list could be felt as an > unnecessary effort, but then: you do need to know the Source module any way. > I have no strict opinion regarding JRE, though. Knowing the module name is fine, but one still has to find the corresponding entry in the UI > > > Advantage of this will be user need not search the Projects/Libraries and > > then provide the properties for it. At one place user can see over all what > > are the properties defined. > > Conceptually its a matrix of module x options. > > You seem to prefer to make a section for options, whereas I see the modules as > the major grouping factor, where everything should be sorted under. > But right now, the grouping is based on the classpath entry kind :-). If the new section presents the options grouped by modules it would be nice (for me as a user). > One more idea: When you think about compatibility with javac & java, wouldn't it > be cool if Build path and launch's Classpath had a button for "previewing" a > suitable commandline for javac / java? > Many times I run a program, and then need to go to the debug view and see which > node has the command line in its properties just to figure out the bare command > line. Having a shortcut for this would be awesome, if you ask me. > > For the issue at hand: such preview feature would allow us, to define the > options in any structure we like and still enable the user to use the configured > options for command line usage. > > > User should be able to provide these options for user defined containers. > > sure > > > Last question: do you see any option that does not relate to any module in a > classpath entry? add-modules and limit-modules are not related to any one entry in particular. add-modules option adds modules to the root modules, and is not the same as adding entries to the module path.
Core semantic questions first: (In reply to Sasikanth Bharadwaj from comment #25) > > Last question: do you see any option that does not relate to any module in a > > classpath entry? > add-modules and limit-modules are not related to any one entry in > particular. add-modules option adds modules to the root modules, and is not > the same as adding entries to the module path. So far, ecj has no concept of root modules, so let's check, if we need to add this and how. For one, javadoc of java.lang.module has this: "The set of root modules, whose names are the initial input to this algorithm, is determined in an implementation specific manner." => We have some freedom here :) JEP 261 distinguishes between compilation for the unnamed module and otherwise: - roots when compiling unnamed: java.se plus some contribution from the upgrade module path. - roots otherwise: "At compile time it is usually the set of modules being compiled" What are the effects of a module being or not being a root module? (a) Starting from root modules, the set of observable modules is filtered to yield a set of reachable modules, which are the members of the module graph. (b) "reads" edges are only ever added for members of the module graph. (c) If any root module is not observable, an error should be raised. Same, if any member of the module graph requires a non-observable module. When is the difference between observable modules and reachable modules (members of the graph) relevant? (d) I can put any number of modules into the environment; as long as they are not reachable, compiler doesn't need to analyse their dependencies. Fine. (e) As per JLS 7.7.5: "An unnamed module reads every observable module". So no relevance of root / reachable here. (f) For automatic modules, javadoc of java.lang.module has the following in Step 2: "If B is an automatic module, then A "reads" every other enumerated automatic module." => Automatic modules don't read modules that may be observable but are not members of the graph ("enumerated"). From these, I think only (c) and (f) produce any user-visible effect. For (c) we should indeed raise errors if the configuration is "incomplete". (f) could create a situation where we look at an automatic module, see a type in its API, but within the module graph we find no module that exports this type (its package). I understand that this is an error even if the desired target module is observable (just not member of the graph). But would this help the user? Meaning: if compilation depends on automatic module A, which needs types from module B, which is on the module path (hence observable), but not reachable from any root module (hence no reads from A to B), why should the user be interested in seeing this as an error? Given we have the freedom to determine the root modules in an implementation specific manner, what would we loose, if we start out by saying: every module on the source path or module path is a root? --limit-modules adds the opposite focus: we want to be sure, we don't inadvertently depend on s.t. (which may not be present at runtime, e.g.). Any modules not contained in any ClasspathEntry are not observable in the first place, so there's nothing to limit. For single-module ClasspathEntries, there is no need to say --limit-modules, easier to just delete the entry :) For any classpath containers, --limit-modules could be used as a (positive) filter, so that only selected modules from the container are considered as observable. Special case JRE: by default on the module path, but by default only java.se is a root, so we could treat the JRE container as if by default having --limit-modules=java.se If our options were to be translated into a legal javac commandline, if --limit-modules is used on one container, the corresponding javac command would need to enumerate all modules mentioned in --limit-modules plus all modules from unconstrained ClasspathEntries. TL;DR I don't see a need to surface a concept of root modules in the UI. add-modules is not needed limit-modules can be defined per classpath container I discussed it with focus on compilation, I may have missed subtle differences regarding runtime. We haven't even started to discuss --upgrade-module-path --patch-module
(In reply to Sasikanth Bharadwaj from comment #25) > Knowing the module name is fine, but one still has to find the corresponding > entry in the UI Yes, this is a key statement we should improve on. How do you find a module you depend on? We already have a similar problem with the package explorer pre Java 9: Where can users conveniently find a particular jar inside, say, "Maven Dependencies" or "Required Plugins"? Currently, that's a PITA for large lists of dependencies. Two crazy ideas to connect the bits: - Support an alternative layout in the package explorer, where we don't show individual classpath containers, but only one container "Required Modules" (with contributions from any ClasspathEntries). This node's children should be sorted alphabetically! (In the module world classpath order is no longer relevant :) ). - Let users edit things like add-exports by selecting the module in the package explorer and opening the module's properties (meaning: the properties of the module as seen from the current project, meaning: the properties of the containing ClasspathEntry concerning this module). Note, that m2e has a dependencies tab in their pom editor, but still users need to have the same information in the package explorer, too. OTOH, also the Java Build Path could possibly offer additional views similar to a pom editor (incl. the distinction between direct and transitive dependencies).
(In reply to Stephan Herrmann from comment #27) > (In reply to Sasikanth Bharadwaj from comment #25) > > Knowing the module name is fine, but one still has to find the corresponding > > entry in the UI > > Yes, this is a key statement we should improve on. How do you find a module > you depend on? If this is about the Libraries tab, see bug 323679. The filter can also be enhanced to filter based on values by using ~value along with the key, similar to the one used in Compiler > Errors/Warnings page, if applicable here.
(In reply to Noopur Gupta from comment #28) > If this is about the Libraries tab, see bug 323679. The filter can also be > enhanced to filter based on values by using ~value along with the key, > similar to the one used in Compiler > Errors/Warnings page, if applicable > here. We will need this both on Projects and Libraries tab I guess.
I think we all are now ok with having these properties for each Library/Project entry with following guidelines (Suggestions welcome) - 1. Entry will have a node for Modular properties. User should be able to define if entry is Modular and then could define properties like export, read, limit etc. Dialog can pre fill the source module etc. 2. JRT needs to be presented as Modular showing the list of modules. 3. As we will have many properties like export, read , it might be difficult to have status for all. We can have just 2 labels - Is a Module - Is not a module 4. Here it will be good to have a label something like Is a module(E,R) --> This module has one exports and one Read Is a module(E+,R,P+) -> This module has more than 1 Exports, 1 Read and more than 1 patch modules 5. User should be able to search for modules with value such as "--add-exports'. 6. Modules can be overlayed with "M" icon unless we group then under Modulepath and Classpath node. @Stephan, Does it sound ok to proceed ?
Thanks, Sarika, for the summary! Let me try to compare with what we have in gerrit: (In reply to Sarika Sinha from comment #30) > I think we all are now ok with having these properties for each > Library/Project entry with following guidelines (Suggestions welcome) - > 1. Entry will have a node for Modular properties. User should be able to > define if entry is Modular and then could define properties like export, > read, limit etc. Dialog can pre fill the source module etc. Conceptually, we have this. Pre-filling in the dialog is still missing, but we agree on that this is desirable. > 2. JRT needs to be presented as Modular showing the list of modules. Not yet, but sounds good. I read the first part of this as requesting that JRT itself has an "M" node, which allows adding add-exports for any of its contained modules. Additionally, each of the contained modules may support adding details to just this module. OK? Or should details only be available via individual modules? I'm fine with either. > 3. As we will have many properties like export, read , it might be difficult > to have status for all. We can have just 2 labels > - Is a Module > - Is not a module If containers like JRT have a toplevel "M" node, then "Is a Module" would be imprecise. What about: - Is modular - Is not modular ? It's the same idiom you used in your text above :) > 4. Here it will be good to have a label something like > Is a module(E,R) --> This module has one exports and one Read > Is a module(E+,R,P+) -> This module has more than 1 Exports, 1 Read and > more than 1 patch modules I agree we shouldn't overwhelm the user. OTOH, I'm not convinced of the proposed shorthand (if that was intended verbatim). What about: Is Modular (modifies encapsulation) for just any occurrence of any of the detail options. This fits all of the options except for --limit-modules :-/ So: - (modifies encapsulation) - (limits content) - (limits content & modifies encapsulation) ? BTW: should --limit-modules also reduce the list of contained modules in the tree? I believe so. > 5. User should be able to search for modules with value such as > "--add-exports'. I read this as a detail request for bug 323679, right? > 6. Modules can be overlayed with "M" icon unless we group then under > Modulepath and Classpath node. I see bug 520300 scheduled for BETA_J9, so the "unless" part should be seen as true? :) > @Stephan, > Does it sound ok to proceed ? Sounds great (pending answers to above detail questions). Technically, it would be great to first decide in bug 520300 and then resolve this current bug on top of that.
(In reply to Stephan Herrmann from comment #31) > > 1. Entry will have a node for Modular properties. User should be able to > > define if entry is Modular and then could define properties like export, > > read, limit etc. Dialog can pre fill the source module etc. > > Conceptually, we have this. Pre-filling in the dialog is still missing, but > we agree on that this is desirable. Yes > > > 2. JRT needs to be presented as Modular showing the list of modules. > > Not yet, but sounds good. > I read the first part of this as requesting that JRT itself has an "M" node, > which allows adding add-exports for any of its contained modules. > Additionally, each of the contained modules may support adding details to > just this module. OK? > Or should details only be available via individual modules? > I'm fine with either. I am also fine with either but even though it might not be correct literally that the container is modular, but it can denote a modular container as it can have only modules. > > 3. As we will have many properties like export, read , it might be difficult > > to have status for all. We can have just 2 labels > > - Is a Module > > - Is not a module > > If containers like JRT have a toplevel "M" node, then "Is a Module" would be > imprecise. What about: > - Is modular > - Is not modular > ? > It's the same idiom you used in your text above :) > > > > 4. Here it will be good to have a label something like > > Is a module(E,R) --> This module has one exports and one Read > > Is a module(E+,R,P+) -> This module has more than 1 Exports, 1 Read and > > more than 1 patch modules > > I agree we shouldn't overwhelm the user. OTOH, I'm not convinced of the > proposed shorthand (if that was intended verbatim). > What about: > Is Modular (modifies encapsulation) > for just any occurrence of any of the detail options. > This fits all of the options except for --limit-modules :-/ > So: > - (modifies encapsulation) > - (limits content) > - (limits content & modifies encapsulation) > ? Sounds ok. > > BTW: should --limit-modules also reduce the list of contained modules in the > tree? I believe so. That will certainly be good. > > > > 5. User should be able to search for modules with value such as > > "--add-exports'. > > I read this as a detail request for bug 323679, right? yes > > > > 6. Modules can be overlayed with "M" icon unless we group then under > > Modulepath and Classpath node. > > I see bug 520300 scheduled for BETA_J9, so the "unless" part should be seen > as true? :) Yes, we are trying for it. > > > > @Stephan, > > Does it sound ok to proceed ? > > Sounds great (pending answers to above detail questions). > > Technically, it would be great to first decide in bug 520300 and then > resolve this current bug on top of that.
I'm planning to update the patch to match to our agreement over the weekend.
https://git.eclipse.org/r/#/c/102544/3..4 (patch set #4) contains a next batch of changes (and is rebased on HEAD, so the difference to #3 contains unrelated changes, too :-/ ) I called out a few technical changes in gerrit. Functional changes: - Simplified labels (no more singular/plural distinction), see prev. comment. - Source module is pre-filled as the module defined in the CP entry - Target module is pre-filled as the current project's module - Content assist proposes packages from the source module Tested for project dependencies as well as libraries (internal / external jar). I'd like to call out one technical limitation: when you add a new CP entry, you will immediately be able to added modular details, *but* at that point, pre-filling the source module and content assist for packages don't yet work, because the entry is not yet on its project's classpath :(. I currently show the dialog with a warning "Containing build path entry has not yet been persisted. Content assist for details is not available." I'm afraid we'd need new API from JDT/Core to read the module name and packages from a library that doesn't yet have an IJavaElement associated. Once you press "Apply" the limitation is gone, but I don't think we should automatically perform apply, (1) because it would make "Cancel" complex, because we'd need to roll back the implicit apply, and (2) on large projects/workspaces changing the classpath could trigger significant amount of rebuilding. Is the current impl. OK? Would people prefer a question like this?: "Do you want to apply pending changes to enable content assist for details?" [Apply] [Continue without applying] As a separate dialog? As a detail on the AddExports dialog? Not yet: setting "editable= false;" for source and target module fields, if no other choice makes sense. Not yet: JRE container. Not yet: externalizing some new strings (e.g., the above warning message).
Patch set #5 changes how a Java 9 JRE Container is rendered: Instead of showing the jrt-fs.jar pseudo entry, the tree now lists the contained modules, nice - but: Now we have a clash between: - (M) Is modular - (M) java.base - (M) ... I.e., the same Module icon is used for two different purposes. At the current stage the first of these nodes is actually necessary, because so far that's the only place where adding add-exports is supported, the details of each of the modules say: "Is modular - unmodifiable". Any ideas for a nicer design? My first idea was to push the "real" modules down as children of "Is modular", but (a) this requires some not-so-nice hacks in the implementation, (b) this makes the modules less visible, even less than the individual jars in a pre-9 JRE. It seems we have technical and conceptual difficulties based on the fact that internally we distinguish between elements and attributes, whereas the UI merges attributes and child elements to a flat list.
Patch set #6 adds more content assist for Java 9 JRE container: Source Module: propose from all contained modules Package: proposed from packages contained in any module in the container. If Source Module is set, that value is used to filter the list of packages. If source module contains an invalid/unmatched name, no filtering is applied. Implementation note: I tried to avoid the deprecated ISubjectControlContentAssistProcessor but found that the "replacement" via jface's IContentProposalProvider is inferior, because IContentProposal does not support images to begin with (SimpleContentProposalProvider even repeats the already type part). So I created JavaPrecomputedNamesAssistProcessor in the tradition of processors like JavaPackageCompletionProcessor. I hope the placement in ui.refactoring is OK, even though it is not (yet?) used by refactoring.
New Gerrit change created: https://git.eclipse.org/r/103733
(In reply to Eclipse Genie from comment #37) > New Gerrit change created: https://git.eclipse.org/r/103733 While testing in a runtime workbench I was reminded that no classpath entry should have multiple extra attributes of the same name, hence multiple --add-exports must be encoded within a single attribute => the value is now interpreted as a ':' separated list. The new gerrit contains the Core part for this.
Gerrit change https://git.eclipse.org/r/103733 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=10dd02010dd61fe266fb956c77e9e4e93f61367c
New in https://git.eclipse.org/r/#/c/102544/6..7 Decode / encode multiple add-exports in the value of a single classpath attribute to match comment 38. Reflect in the UI that in this workflow we can only add exports towards one target module (the module of the current project).
Patch set #8 is dedicated to validation and widget enablement etc. Validation: - Source module must be served by the classpath entry - Package must be served by the source module - Source module problem has priority - Initially no error shown, but OK disabled - When Source module is OK, but package empty, no error but OK disabled - When changing source invalidates package, switch focus to package Short-hand: we have exactly one checkbox, labeled "Defines one or more modules", which toggles the "Is modular" flag => will be called "the checkbox" below. Enablement: - When classpath entry is multi-module, assume this is JRE9 and disable the checkbox (cannot be made non-modular) - When the checkbox is unchecked, disable add/edit/remove buttons - Source module field if pre-filled is disabled. - Target module field is always pre-filled and disabled. - OK initially disabled, see above. Update: When the checkbox is being unchecked, all defined add-exports are removed. Not yet: check for duplicate packages.
Reminder for self: still need to address the original request regarding ALL-UNNAMED (as Target module in case the current project is not modular).
Another idea: adding exports to JRE, when s.o. selects a Package, we could update the Source module to the correct module containing that package.
Thanks Stephan!! Tried the patch, looks good. Some answers - >Once you press "Apply" the limitation is gone, but I don't think we should >automatically perform apply, (1) because it would make "Cancel" complex, because >we'd need to roll back the implicit apply, and (2) on large projects/workspaces >changing the classpath could trigger significant amount of rebuilding. > >Is the current impl. OK? >Would people prefer a question like this?: > "Do you want to apply pending changes to enable content assist for details?" > [Apply] [Continue without applying] >As a separate dialog? As a detail on the AddExports dialog? Yes, a separate dialog on click of Edit will help the user to know that apply is required and if user still chooses to Continue without applying, we can show the warning that content assist is not available, Please apply at BuildPath Page. >Another idea: adding exports to JRE, when s.o. selects a Package, we could >update the Source module to the correct module containing that package. Sounds good. >I hope the placement in ui.refactoring is OK, even though it is not (yet?) used >by refactoring. Noopur will confirm. Few observations - 1. Same icon M for modules and "Is modular" may give wrong impression. Some other icon for Encapsulation property will be better. 2. Haven't tested all cases but got this NPE during "Edit" for "Is Modular" after adding "Class Folder".
May be we can filer the source modules from content assist if it does not have any package like jdk.pack
(In reply to Sarika Sinha from comment #44) > >I hope the placement in ui.refactoring is OK, even though it is not (yet?) used >by refactoring. > Noopur will confirm. Yes, that would be OK.
(In reply to Sarika Sinha from comment #44) > 2. Haven't tested all cases but got this NPE during "Edit" for "Is Modular" > after adding "Class Folder". I see the NPE when I click "Edit" and then "Add". Several contributing factors: - ModuleAddExport.getTargetJavaElements(IJavaProject, IPath) fails to test existence in one branch and effectively returns "new IJavaElement[] {null};" - that null will not be handled, it should be avoided right here by answering null instead of the array. - The reason we get is, that before "Apply" the class folder is not yet on the classpath, just like previously mentioned. - Even after "Apply" the ModuleClassFile does not provide an IModuleDescription, because of an incompleteness from bug 520651. Thanks for catching this!
The next installment (#9) brings these: - based on the Core fix from bug 520651 comment 46 we can now handle class folders, too. The UI part now correctly catches the null problem mentioned in comment 47. - added the question dialog discussed in comment 44: [Add to project now] [Proceed without adding] [Cancel] I opted for only adding the current entry, not applying all pending changes, OK? - shortened the warning an the inner "Add" dialog to fit in the dialog (looks OK on linux, how about other platforms?) Looks like we have two open issues regarding icons: - the (M)/(M) clash discussed in comment 35 & comment 44 - the new icon "export_package.png" probably needs to be re-done via svg. Looking into the future (with more options similar to add-exports) we may also want to invent one icon for all those "modifications of encapsulation". If we have a nice one here, we could perhaps use it for the parent node ("Is modular"), too? Something based on the module icon but with some tool applied to it? A screwdriver that opens the module? :) OTOH, if modification is dominant it is no longer suitable for a plain "Is modular" without modifications ... Any volunteers for helping with these icon issues? :)
(In reply to Sarika Sinha from comment #45) > May be we can filer the source modules from content assist if it does not > have any package like jdk.pack I'm not 100% which scenario this refers to. Is it about first entering a package and then switching to the "Source module" field? If the user already selected the package, we can fully auto-insert the associated source module, so I don't see a need for content assist here. Am I missing s.t.? (comment 43 - this is not yet implemented, though). Collecting the open issues I'm aware of: - auto-fill module from given package - icons - ALL-UNNAMED - check for duplicate packages
(In reply to Stephan Herrmann from comment #49) > (In reply to Sarika Sinha from comment #45) > > May be we can filer the source modules from content assist if it does not > > have any package like jdk.pack > > I'm not 100% which scenario this refers to. Is it about first entering a > package and then switching to the "Source module" field? > This scenario is when I selected jdk.pack in the Source module, and i tried content assist for Package, there are no results. So in that case case should we not filter out jdt.pack from the list provided in the Source Module via Content assist. >I opted for only adding the current entry, not applying all pending changes, OK? Ok > shortened the warning an the inner "Add" dialog to fit in the dialog > (looks OK on linux, how about other platforms?) looks ok on Windows as well
I don't have exact steps but I got this - java.lang.NullPointerException at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleAddExportsBlock.moduleNames(ModuleAddExportsBlock.java:131) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleAddExportsBlock.computeSourceModuleStatus(ModuleAddExportsBlock.java:346) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleAddExportsBlock.updateModuleStatus(ModuleAddExportsBlock.java:333) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleAddExportsBlock.addExportsDialogFieldChanged(ModuleAddExportsBlock.java:324) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleAddExportsBlock.lambda$0(ModuleAddExportsBlock.java:99) at org.eclipse.jdt.internal.ui.wizards.dialogfields.DialogField.dialogFieldChanged(DialogField.java:75) at org.eclipse.jdt.internal.ui.wizards.dialogfields.StringDialogField.doModifyText(StringDialogField.java:146) at org.eclipse.jdt.internal.ui.wizards.dialogfields.StringDialogField.access$0(StringDialogField.java:142) at org.eclipse.jdt.internal.ui.wizards.dialogfields.StringDialogField$1.modifyText(StringDialogField.java:113) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:180) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:86) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4428) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1103) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1084) at org.eclipse.swt.widgets.Text.wmCommandChild(Text.java:3122) at org.eclipse.swt.widgets.Control.WM_COMMAND(Control.java:4991) at org.eclipse.swt.widgets.Control.windowProc(Control.java:4846) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5191) at org.eclipse.swt.internal.win32.OS.CallWindowProcW(Native Method) at org.eclipse.swt.internal.win32.OS.CallWindowProc(OS.java:2454) at org.eclipse.swt.widgets.Text.callWindowProc(Text.java:262) at org.eclipse.swt.widgets.Control.windowProc(Control.java:4941) at org.eclipse.swt.widgets.Text.windowProc(Text.java:2704) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5178) at org.eclipse.swt.internal.win32.OS.DispatchMessageW(Native Method) at org.eclipse.swt.internal.win32.OS.DispatchMessage(OS.java:2560) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3815) at org.eclipse.jface.window.Window.runEventLoop(Window.java:818) at org.eclipse.jface.window.Window.open(Window.java:794) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleDialog.addEntry(ModuleDialog.java:280) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleDialog.doCustomButtonPressed(ModuleDialog.java:239) at org.eclipse.jdt.internal.ui.wizards.buildpaths.ModuleDialog$AddExportsAdapter.customButtonPressed(ModuleDialog.java:327) at org.eclipse.jdt.internal.ui.wizards.dialogfields.ListDialogField.buttonPressed(ListDialogField.java:209) at org.eclipse.jdt.internal.ui.wizards.dialogfields.ListDialogField.doButtonSelected(ListDialogField.java:480) at org.eclipse.jdt.internal.ui.wizards.dialogfields.ListDialogField.access$0(ListDialogField.java:476) at org.eclipse.jdt.internal.ui.wizards.dialogfields.ListDialogField$2.widgetSelected(ListDialogField.java:442) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:249) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:86) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4428) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4238) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3817) at org.eclipse.jface.window.Window.runEventLoop(Window.java:818) at org.eclipse.jface.window.Window.open(Window.java:794) at org.eclipse.jdt.internal.ui.wizards.buildpaths.BuildPathBasePage.showAddExportDialog(BuildPathBasePage.java:108) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.editAttributeEntry(LibrariesWorkbookPage.java:521) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.editEntry(LibrariesWorkbookPage.java:477) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.libaryPageCustomButtonPressed(LibrariesWorkbookPage.java:269) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage.access$0(LibrariesWorkbookPage.java:247) at org.eclipse.jdt.internal.ui.wizards.buildpaths.LibrariesWorkbookPage$LibrariesAdapter.customButtonPressed(LibrariesWorkbookPage.java:204) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField.buttonPressed(TreeListDialogField.java:171) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField.doButtonSelected(TreeListDialogField.java:402) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField.access$2(TreeListDialogField.java:398) at org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField$2.widgetSelected(TreeListDialogField.java:363) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:249) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:86) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4428) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4238) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3817) at org.eclipse.jface.window.Window.runEventLoop(Window.java:818) at org.eclipse.jface.window.Window.open(Window.java:794) at org.eclipse.ui.dialogs.PropertyDialogAction.run(PropertyDialogAction.java:157) at org.eclipse.jface.action.Action.runWithEvent(Action.java:473) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:565) at org.eclipse.jface.action.ActionContributionItem.lambda$4(ActionContributionItem.java:397) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:86) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4428) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4238) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3817) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1150) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1039) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:680) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:594) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:148) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:151) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:673) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:610) at org.eclipse.equinox.launcher.Main.run(Main.java:1519) at org.eclipse.equinox.launcher.Main.main(Main.java:1492)
Patch set #10 fixes the remaining implementation issues from my agenda: - Support ALL-UNNAMED (auto-filled if current project is not modular) - Error "Duplicate package {0}" in the first level dialog (list) - When Source module is empty, selecting a package also sets the Source module This is actually a nice way to learn the modules: type a package, see its module :) - NPE from comment 51 I'm afraid the request from comment 50 is a bit expensive: search all package fragment roots if they have any package fragments that have any java elements (CUs). At least the current impl. is transparent: we don't run risk to confuse a user who might know about a module and not find it in the list of proposals. If desired we can still add it via its own bug, later. I have not worked on the icon issue, and honestly I'd appreciate if s.o. else steps in, or at least proposes something. At this point I'd suggest to push the current state and handle icons and any remaining problems in follow-up bugs (among those follow-ups: more options, like --limit-modules; apply the solution to launch configurations, where we need to also support --add-opens ...) Any objections?
*** Bug 521355 has been marked as a duplicate of this bug. ***
Looks good!! We can have follow up bugs to add more details.
Gerrit change https://git.eclipse.org/r/102544 was merged to [BETA_JAVA9]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=46485a8139ffed6862fd7742c179a71cf2e34a20
(In reply to Eclipse Genie from comment #55) > Gerrit change https://git.eclipse.org/r/102544 was merged to [BETA_JAVA9]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=46485a8139ffed6862fd7742c179a71cf2e34a20 > Released for BETA_JAVA9, thanks. Follow-up bugs so far: bug 521662 and bug 521663
Created Bug 521666 for --add-reads and --limit-modules in IDE.
Thanks, Stephan and Sarika. I looked at the feature after pulling the latest changes and it looks good. I will be raising any issues that I find in separate bugs and will add a dependency to this one.
