Community
Participate
Working Groups
Bug 533619 added the command line option to enable --enable-preview. We need an UI option to be able to add this, this is must for JEP 325: Switch Expressions Workaround, org.eclipse.jdt.core.compiler.problem.EnablePreviews=enabled needs to be added in org.eclipse.jdt.core.prefs
We can add this option in the Java Compiler preference page so that it can be configured on the workspace and project level also. It can be added as a checkbox option after "Use default compliance settings". By default, this "Enable preview" option will be unchecked, similar to the command line which needs to add it explicitly to enable the preview. It will be enabled only if the compliance level is set to 11 or higher. We can also add a quick fix to enable the preview checkbox later. It will need a new API in JDT Core in org.eclipse.jdt.core.JavaCore. JDT Debug (bug 542580) can read this API value. If anyone has a better suggestion for this option, then please suggest.
> It can be added as a checkbox option after "Use default compliance settings". How about adding it to the compliance sub-options? If not, I think we should not only add this new option below the compliance settings but also the "Use '--release option".
This was available for Java 11 and we can put the feature in master itself.
(In reply to Dani Megert from comment #2) > How about adding it to the compliance sub-options? +1 because enable-preview option is always tied to compliance. In long, try the following in the command line: $ javac12 --enable-preview X.java error: --enable-preview must be used with either -source or --release and the correct usage is: $ javac12 --source 12 --enable-preview X.java Since there is no stand-alone existence for –enable-preview option in command line, it would do well to provide that idea in IDE as well.
Created attachment 276993 [details] Screenshot - checkbox As per my understanding, the attached screenshot shows the location where the new option is expected. Please let me know if a change is required.
See bug 542654 comment #6 also to decide the UI.
(In reply to Noopur Gupta from comment #6) > See bug 542654 comment #6 also to decide the UI. Based on bug 542654 comment #9, we will have the checkbox as discussed in this bug.
(In reply to Dani Megert from comment #2) > > It can be added as a checkbox option after "Use default compliance settings". > > How about adding it to the compliance sub-options? If not, I think we should > not only add this new option below the compliance settings but also the "Use > '--release option". Enabling/disabling --release option has an effect on the following default compliance setting. So, I am not so sure about placing it below. I am fine with Preview option being placed below that, though.
New Gerrit change created: https://git.eclipse.org/r/134773
(In reply to Jay Arthanareeswaran from comment #8) > I am fine with Preview option being placed below that, though. Do you mean like in comment #5? On second thoughts, shouldn't the option be similar to the existing "Disallow identifiers called 'assert'? It can be called "Disallow preview features". It will be enabled only when compliance is >= 11. The default value in the drop-down will be "Error" which is the same as disabling preview features. The value "Ignore" will be equivalent to enabling preview features. Otherwise, having a checkbox here and a drop-down in Errors/Warnings page (under a new category on that page?) might be confusing. Let me know your thoughts.
(In reply to Noopur Gupta from comment #10) > Otherwise, having a checkbox here and a drop-down in Errors/Warnings page > (under a new category on that page?) might be confusing. > > Let me know your thoughts. I don't think we have a choice. We have to have a switch to tell the compiler to support preview features. And (only) when enabled, the project can not only be configured to report the usage of a preview feature, but also suppress with @SuppressWarnings("preview") in case of a warning severity :) For the latter, the best place I can think of in the Errors/Warnings page the "Deprecated and restricted API" section. But one can argue that we are not really talking about deprecated stuff.
Calling it "disallow" would only be an option if the default is "on" in Java, which I think it is not. In my opinion we need two options, one to enable/disable preview features and one to set the severity for preview feature code when the first option is off. We can add a link below the new option on the Compiler page that takes the user to the severity option on the Errors/Warnings page. Side note: While not right now, there might be a new option to enable/disable a concrete preview feature.
(In reply to Noopur Gupta from comment #10) > (In reply to Jay Arthanareeswaran from comment #8) > > I am fine with Preview option being placed below that, though. > Do you mean like in comment #5? > > > On second thoughts, shouldn't the option be similar to the existing > "Disallow identifiers called 'assert'? > > It can be called "Disallow preview features". (In reply to Dani Megert from comment #12) > Calling it "disallow" would only be an option if the default is "on" in > Java, which I think it is not. Sorry, I wanted to propose "Allow preview features".
(In reply to Dani Megert from comment #12) > Calling it "disallow" would only be an option if the default is "on" in > Java, which I think it is not. > > In my opinion we need two options, one to enable/disable preview features > and one to set the severity for preview feature code when the first option > is off. > > We can add a link below the new option on the Compiler page that takes the > user to the severity option on the Errors/Warnings page. > > Side note: While not right now, there might be a new option to > enable/disable a concrete preview feature. Regarding the severity for preview feature: I assume that in many cases when preview code is used but the feature is off it would be a compile error which can't be set to 'Warning' or 'Ignore'. So, for which cases would we need the option? Maybe different options are needed depending on the preview feature (see my side note in comment 12.
(In reply to Dani Megert from comment #12) > In my opinion we need two options, one to enable/disable preview features > and one to set the severity for preview feature code when the first option > is off. Looks like there is some misunderstanding. My expectation is, when the first option is off, we always report an error. Not at all configurable. The second option I am talking about is when preview features are enabled. The user can still opt to be notified through configurable error preference. This from the relevant JDK bug: ----- If the --enable-preview flag is enabled, the behavior of javac should change as follows: javac should additionally recognize preview features ... javac must be able to warn (where possible) of any use of preview features, as a measure to notify users that they are relying on features that might be removed in subsequent releases. Such warnings should be reported under the 'preview' Lint category - which will be added as part of this change. Therefore, developers will have the ability to suppress such warnings in the usual way (e.g. via @SuppressWarnings("preview") or using the command line flag -Xlint:-preview). ------
(In reply to Jay Arthanareeswaran from comment #15) > This from the relevant JDK bug: This is the bug: https://bugs.openjdk.java.net/browse/JDK-8200312
I thing we want the option to be outside and below "use default compliance settings". Looks like we have 2 approaches for the option - 1. Have 1 dropdown -"Use preview features" with default set to Error, user can change to Warning/Info/Ignore. Error will mean Disable preview feature, any other value will mean enabled with the chosen severity. 2. Have 1 checkbox in Compiler page to indicate enable/disable. And Error/Warnings page will have 1 dropdown with "Error" as default when checkbox "Enable Preview" is unchecked. When the user checks this option on Compiler page, the dropdown should change to Warning by default and user can change it to Info/Ignore with preview checkbox enabled. If user changes the dropdown to error, the "Enable preview" should be unchecked automatically.
(In reply to Sarika Sinha from comment #17) > I thing we want the option to be outside and below "use default compliance > settings". > > Looks like we have 2 approaches for the option - I think it will confuse the user if the same drop down is used to enable/disable a preview feature. I would go with a simple check box, which activates/deactivates the drop down with warning/info/ignore severity options. This also depicts what the compiler is doing internally and closer to the command line options. As a side note, this checkbox can be used to automatically pass the --enable-preview VM argument via the run configuration. Just thinking aloud.
(In reply to Sarika Sinha from comment #17) > I thing we want the option to be outside and below "use default compliance > settings". Just to confirm, this means that the option will be a sibling of --release and "Use default compliance settings" checkboxes, which will be placed as the last option in the "JDK Compliance" section. Do we agree on this? See comment #4 also. > Looks like we have 2 approaches for the option - I am fine with any of the two approaches.
(In reply to Jay Arthanareeswaran from comment #18) > I would go with a simple check box, which > activates/deactivates the drop down with warning/info/ignore severity > options. In that case, the Javadoc of the API JavaCore.COMPILER_PB_REPORT_PREVIEW_FEATURES should be updated. Using "error" in the description and possible values makes things confusing. Based on the above comment, I understand that "error" is not an option for this API.
(In reply to Noopur Gupta from comment #20) > In that case, the Javadoc of the API > JavaCore.COMPILER_PB_REPORT_PREVIEW_FEATURES should be updated. Using > "error" in the description and possible values makes things confusing. Based > on the above comment, I understand that "error" is not an option for this > API. I wasn't completely sure when I left out "error". I just checked and looks like we have no option that doesn't have error as a severity. Should we just keep this the same way? One might question what is the difference between simply disabling Preview and enable but setting this severity to error. The difference would be, in the latter case the preview feature will still be compiled, albeit with an additional error.
(In reply to Jay Arthanareeswaran from comment #21) > (In reply to Noopur Gupta from comment #20) > > In that case, the Javadoc of the API > > JavaCore.COMPILER_PB_REPORT_PREVIEW_FEATURES should be updated. Using > > "error" in the description and possible values makes things confusing. Based > > on the above comment, I understand that "error" is not an option for this > > API. > > I wasn't completely sure when I left out "error". I just checked and looks > like we have no option that doesn't have error as a severity. Should we just > keep this the same way? We don't have any other option that is enabled/disabled by a checkbox on the Compiler page also. We can treat this case differently. > One might question what is the difference between simply disabling Preview > and enable but setting this severity to error. The difference would be, in > the latter case the preview feature will still be compiled, albeit with an > additional error. That's what has been confusing me since the beginning. I think having a checkbox to enable/disable preview features on the Compiler page and a drop-down on the Errors/Warnings page to set the severity (Warning/Info/Ignore) when the preview features are enabled by the checkbox, will be the least confusing of all the options discussed above.
(In reply to Noopur Gupta from comment #19) > (In reply to Sarika Sinha from comment #17) > > I thing we want the option to be outside and below "use default compliance > > settings". > Just to confirm, this means that the option will be a sibling of --release > and "Use default compliance settings" checkboxes, which will be placed as > the last option in the "JDK Compliance" section. > > Do we agree on this? See comment #4 also. This still remains unanswered. Manoj, do you have any input based on your comment #4?
Gerrit change https://git.eclipse.org/r/134773 was merged to [BETA_JAVA_12]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=4721c59c471c63b7fffa49daef959f2b4e0375be
(In reply to Eclipse Genie from comment #24) > Gerrit change https://git.eclipse.org/r/134773 was merged to [BETA_JAVA_12]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.debug.git/commit/?id=4721c59c471c63b7fffa49daef959f2b4e0375be This adds the two APIs with their default values to Java profile properties in jdt.launching.
Here's what I think we should do: - Add 'Enable preview features' to default compliance settings at the top. - Add a sub-option combo box to set the severity for used preview features. - The combo box will have 'Warning', 'Info' and 'Ignore', default: 'Warning'.
(In reply to Dani Megert from comment #26) > Here's what I think we should do: > - Add 'Enable preview features' to default compliance settings at the top. > - Add a sub-option combo box to set the severity for used preview features. > - The combo box will have 'Warning', 'Info' and 'Ignore', default: 'Warning'. +1, although I am not very particular about the Enabled Preview checkbox at the top.
(In reply to Jay Arthanareeswaran from comment #27) > (In reply to Dani Megert from comment #26) > > Here's what I think we should do: > > - Add 'Enable preview features' to default compliance settings at the top. > > - Add a sub-option combo box to set the severity for used preview features. > > - The combo box will have 'Warning', 'Info' and 'Ignore', default: 'Warning'. > > +1, although I am not very particular about the Enabled Preview checkbox at > the top. My reasoning to put it at the top is because it will be enabled when --release is checked. The combo boxes below will remain disabled in that scenario. Also, it looks nicer if the check box is not in-between combo boxes.
Created attachment 277192 [details] Screenshot - checkbox & combo Please check if this is the expected UI. Based on the side note in comment #12, I am not sure how this will look when multiple features are added.
(In reply to Noopur Gupta from comment #29) > Created attachment 277192 [details] > Screenshot - checkbox & combo > > Please check if this is the expected UI. +1 > Based on the side note in comment #12, I am not sure how this will look when > multiple features are added. We will have to deal with this when it happens. A preview feature might also come with new optional problems which we will have to show on the Errors/Warnings page.
New Gerrit change created: https://git.eclipse.org/r/135608
(In reply to Eclipse Genie from comment #31) > New Gerrit change created: https://git.eclipse.org/r/135608 WIP patch for UI.
(In reply to Eclipse Genie from comment #31) > New Gerrit change created: https://git.eclipse.org/r/135608 I think we need to explain the severity level preference better. We need to convey that using preview language features will be reported/highlighted with this preference.
(In reply to Jay Arthanareeswaran from comment #33) > (In reply to Eclipse Genie from comment #31) > > New Gerrit change created: https://git.eclipse.org/r/135608 > > I think we need to explain the severity level preference better. We need to > convey that using preview language features will be reported/highlighted > with this preference. Currently, the checkbox label is: Enable preview features And, the combo box label is: Preview features with severity level: Do you have a better suggestion for the combo box label? Or, should we describe this behavior in the F1 help documentation?
Bug 543772 will add the F1 help doc directly into the master branch of platform.common repo after the BETA branches are merged to master.
Gerrit change https://git.eclipse.org/r/135608 was merged to [BETA_JAVA_12]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=b823e82835414ca8e5e65074df4d53f946c64310
(In reply to Eclipse Genie from comment #36) > Gerrit change https://git.eclipse.org/r/135608 was merged to [BETA_JAVA_12]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=b823e82835414ca8e5e65074df4d53f946c64310 Released for BETA_JAVA_12 branch. Label can be updated if required. Please open new bugs for any enhancements.
The quickfix for Configure problem severity opens up the Errors/Warning page, Should it not open up the Java Compiler page?
(In reply to Sarika Sinha from comment #38) > The quickfix for Configure problem severity opens up the Errors/Warning > page, Should it not open up the Java Compiler page? For which problem? We don't have a QF to configure problem severity for preview features yet. It will be implemented in bug 543669.
Can we give 2 quick fixes initially - 1. Enable preview with warning 2. Enable preview with Ignore User has to perform 2 actions without this.
(In reply to Sarika Sinha from comment #40) > Can we give 2 quick fixes initially - > 1. Enable preview with warning > 2. Enable preview with Ignore > > User has to perform 2 actions without this. This should be discussed in a new bug report.
(In reply to Dani Megert from comment #41) > (In reply to Sarika Sinha from comment #40) > > Can we give 2 quick fixes initially - > > 1. Enable preview with warning > > 2. Enable preview with Ignore > > > > User has to perform 2 actions without this. > This should be discussed in a new bug report. Create Bug 545987.
New Gerrit change created: https://git.eclipse.org/r/140091
Gerrit change https://git.eclipse.org/r/140091 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=36d3470701a1c441100ec02b65733e73cb251f78
