You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/10/07 09:20:35 UTC
[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2424: Modularizing Javascript/HTML
JaroslavTulach commented on a change in pull request #2424:
URL: https://github.com/apache/netbeans/pull/2424#discussion_r500833465
##########
File path: java/java.editor/module-auto-deps.xml
##########
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<!DOCTYPE transformations PUBLIC "-//NetBeans//DTD Module Automatic Dependencies 1.0//EN" "http://www.netbeans.org/dtds/module-auto-deps-1_0.dtd">
+
+<transformations version="1.0">
+ <transformationgroup>
+ <description>Removed dependencies on pre-6.5 formatting</description>
+ <transformation>
+ <trigger-dependency type="older">
+ <module-dependency codenamebase="org.netbeans.modules.java.editor" major="1" spec="2.78.0"/>
+ </trigger-dependency>
+ <implies>
+ <result>
Review comment:
One deprecated module less, in a compatible way! Thanks, Sváťo, I always knew you are the best when it comes to module dependencies cleanup!
##########
File path: java/java.editor.lib/src/org/netbeans/editor/ext/java/JavaFoldManager.java
##########
@@ -22,35 +22,29 @@
import org.netbeans.api.editor.fold.FoldType;
import org.netbeans.spi.editor.fold.FoldManager;
-import static org.netbeans.editor.ext.java.Bundle.*;
-import org.openide.util.NbBundle;
+import org.netbeans.modules.java.editor.fold.JavaElementFoldManager;
/**
* Java fold maintainer creates and updates folds for java sources.
*
* @author Miloslav Metelka
* @version 1.00
+ * @deprecated This package introduces a dependency on an obsoleted pre-NetBeans 6.5 features.
+ * It is possible to use generic {@link FoldType} categories and use {@link FoldType#isKindOf(org.netbeans.api.editor.fold.FoldType)}.
Review comment:
Great!
##########
File path: webcommon/javascript2.nodejs/nbproject/project.xml
##########
@@ -78,15 +78,6 @@
<specification-version>1.0</specification-version>
</run-dependency>
</dependency>
- <dependency>
- <code-name-base>org.netbeans.modules.html.editor</code-name-base>
- <build-prerequisite/>
- <compile-dependency/>
- <run-dependency>
- <release-version>2</release-version>
- <specification-version>2.50</specification-version>
- </run-dependency>
- </dependency>
Review comment:
`javascript2.nodejs` module made more "lightweight", great!
##########
File path: ide/projectui/src/org/netbeans/modules/project/ui/OpenProjectList.java
##########
@@ -595,7 +606,7 @@ public boolean isDone() {
@Override
public Project[] get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException {
long ms = unit.convert(timeout, TimeUnit.MILLISECONDS);
- if (!TASK.waitFinished(timeout)) {
+ if (!waitFinished(timeout)) {
Review comment:
This is the important change. Unit tests that call `OpenProjectList.getDefault().get()` could wait indefinitely long. Now they trigger the actual loading mechanism by calling `waitFinished` and not `TASK.waitFinished`. As a result it is more comfortable to write tests that invoke code relying on the list of open projects being loaded.
##########
File path: platform/openide.dialogs/src/org/openide/DialogDisplayer.java
##########
@@ -133,13 +133,19 @@ public Dialog createDialog(DialogDescriptor descriptor, Frame parent) {
* @see "#30031"
*/
private static final class Trivial extends DialogDisplayer {
+ @Override
public Object notify(NotifyDescriptor nd) {
+ if (GraphicsEnvironment.isHeadless()) {
+ return NotifyDescriptor.CLOSED_OPTION;
Review comment:
Showing dialog would yield an exception in headless mode. Returning `CLOSED_OPTION` is the best this default, dummy implementation can do, in my opinion.
##########
File path: ide/csl.api/src/org/netbeans/modules/csl/core/LanguageRegistry.java
##########
@@ -146,32 +146,31 @@ public Language getLanguageByMimeType(@NonNull String mimeType) {
return result;
}
- public List<Language> getEmbeddedLanguages(BaseDocument doc, int offset) {
+ public List<Language> getEmbeddedLanguages(Document doc, int offset) {
List<Language> result = new ArrayList<Language>();
- doc.readLock(); // Read-lock due to Token hierarchy use
- try {
- // TODO - I should only do this for languages which CAN have it
- /*
- at org.netbeans.lib.lexer.inc.IncTokenList.reinit(IncTokenList.java:113)
- at org.netbeans.lib.lexer.TokenHierarchyOperation.setActiveImpl(TokenHierarchyOperation.java:257)
- at org.netbeans.lib.lexer.TokenHierarchyOperation.isActiveImpl(TokenHierarchyOperation.java:308)
- at org.netbeans.lib.lexer.TokenHierarchyOperation.tokenSequence(TokenHierarchyOperation.java:344)
- at org.netbeans.lib.lexer.TokenHierarchyOperation.tokenSequence(TokenHierarchyOperation.java:338)
- at org.netbeans.api.lexer.TokenHierarchy.tokenSequence(TokenHierarchy.java:183)
- at org.netbeans.modules.csl.LanguageRegistry.getEmbeddedLanguages(LanguageRegistry.java:336)
- at org.netbeans.modules.gsfret.hints.infrastructure.SuggestionsTask.getHintsProviderLanguage(SuggestionsTask.java:70)
- at org.netbeans.modules.gsfret.hints.infrastructure.SuggestionsTask.run(SuggestionsTask.java:94)
- at org.netbeans.modules.gsfret.hints.infrastructure.SuggestionsTask.run(SuggestionsTask.java:63)
-[catch] at org.netbeans.napi.gsfret.source.Source$CompilationJob.run(Source.java:1272) *
- */
- TokenSequence ts = TokenHierarchy.get(doc).tokenSequence();
- if (ts != null) {
- addLanguages(result, ts, offset);
+ doc.render(
Review comment:
We tried, with Miloslav Metelka, to make sure all the functionality of `BaseDocument` commonly needed by users of editor is also available via standard `javax.swing.text.Document` or via `NbDocument` abstractions.
It is great to see that such replacements work!
##########
File path: ide/csl.api/apichanges.xml
##########
@@ -25,6 +25,31 @@
<apidef name="csl.api">Common Scripting Language API</apidef>
</apidefs>
<changes>
+ <change id="ExtraMockServices">
+ <summary>Allow subclasses to inject instances to MockLookup</summary>
+ <version major="2" minor="65"/>
+ <date day="30" month="9" year="2020"/>
+ <author login="sdedic"/>
+ <compatibility addition="yes"/>
+ <description>
+ Subclasses of <code>o.n.m.csl.api.test.TestBase</code> may subclass <code>createExtraMockLookupContent</code> to
Review comment:
This could use `<a href="@TOP@/org/netbeans/modules/csl/api/test/TestBase.html">` hyperlink.
##########
File path: ide/csl.api/apichanges.xml
##########
@@ -25,6 +25,31 @@
<apidef name="csl.api">Common Scripting Language API</apidef>
</apidefs>
<changes>
+ <change id="ExtraMockServices">
+ <summary>Allow subclasses to inject instances to MockLookup</summary>
+ <version major="2" minor="65"/>
+ <date day="30" month="9" year="2020"/>
+ <author login="sdedic"/>
+ <compatibility addition="yes"/>
+ <description>
+ Subclasses of <code>o.n.m.csl.api.test.TestBase</code> may subclass <code>createExtraMockLookupContent</code> to
+ add extra services into the <code>MockLookup</code> during test setup.
+ </description>
+ <class package="org.netbeans.modules.csl.api.test" name="TestBase" link="no"/>
+ </change>
+ <change id="Remove-BaseDocument">
+ <summary>BaseDocument references reduced</summary>
+ <version major="2" minor="65"/>
+ <date day="30" month="9" year="2020"/>
+ <author login="sdedic"/>
+ <compatibility addition="yes" deprecation="yes"/>
+ <description>
+ Decprecated API dependency on BaseDocument that introduces (unnecessary) dependency on <code>editor.lib</code> module
Review comment:
Replacing `BaseDocument` with JDK's `Document` decreases coupling and increases flexibility.
##########
File path: platform/sendopts/src/org/netbeans/modules/sendopts/DefaultProcessor.java
##########
@@ -191,6 +191,8 @@ protected void process(Env env, Map<Option, String[]> optionValues) throws Comma
if (inst instanceof ArgsProcessor) {
((ArgsProcessor)inst).process(env);
}
+ } catch (CommandException exception) {
Review comment:
Good catch. There is no point in wrapping `CommandException` into another `CommandException`. Thanks.
##########
File path: ide/html.editor/module-auto-deps.xml
##########
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+
+<!DOCTYPE transformations PUBLIC "-//NetBeans//DTD Module Automatic Dependencies 1.0//EN" "http://www.netbeans.org/dtds/module-auto-deps-1_0.dtd">
+
+<transformations version="1.0">
+ <transformationgroup>
+ <description>HtmlIndex API separated</description>
+ <transformation>
+ <trigger-dependency type="older">
+ <module-dependency codenamebase="org.netbeans.modules.html.editor" spec="2.66"/>
+ </trigger-dependency>
+ <implies>
+ <result>
+ <module-dependency codenamebase="org.netbeans.modules.html.indexing"/>
Review comment:
Separating the indexing functionality out of HTML editor module seems reasonable. Thanks Sváťo for making it a compatible change!
##########
File path: java/maven/src/org/netbeans/modules/maven/execute/AbstractMavenExecutor.java
##########
@@ -141,15 +142,13 @@ protected final void processInitialMessage() {
}
protected final void actionStatesAtStart() {
- SwingUtilities.invokeLater(new Runnable() {
- @Override public void run() {
- createNewTabActions();
- tabContext.rerun.setEnabled(false);
- tabContext.rerunDebug.setEnabled(false);
- tabContext.overview.setRoot(null);
- tabContext.resume.setFinder(null);
- tabContext.stop.setEnabled(true);
- }
+ updateUILater(false, () -> {
+ createNewTabActions();
Review comment:
These are typical UI updating actions and it makes no sense to call them when the system is running in headless mode. Thanks for introducing the `updateUILater` (if ever) utility!
##########
File path: webcommon/web.clientproject.api/nbproject/project.xml
##########
@@ -96,12 +96,11 @@
</run-dependency>
</dependency>
<dependency>
- <code-name-base>org.netbeans.modules.html.editor</code-name-base>
+ <code-name-base>org.netbeans.modules.html.indexing</code-name-base>
Review comment:
`web.clientproject` and associated API module made more "lightweight", super!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists