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/02/19 21:07:29 UTC
[GitHub] [netbeans] matthiasblaesing opened a new pull request #1955: Hook
formating/rangeFormating into LSP Client
matthiasblaesing opened a new pull request #1955: Hook formating/rangeFormating into LSP Client
URL: https://github.com/apache/netbeans/pull/1955
I noticed that the typescript editor is lacking formatting. I investigated and
learned, that the LSP protocol in principle supports formatting and the
typescript language server also contains the necessary support.
This changeset hooks the _format_ and _rangeFormat_ functions into the
IDE.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
[GitHub] [netbeans] matthiasblaesing commented on issue #1955: Hook
formating/rangeFormating into LSP Client
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on issue #1955: Hook formating/rangeFormating into LSP Client
URL: https://github.com/apache/netbeans/pull/1955#issuecomment-589859469
@jlahoda thank you for your input. I pushed an update, that tries to implement the suggested changes. For applyTestEdits I decided to use the simple variant (start and end index instead of a BiPredicate) - I have not a clear vision where a more complex predicate than the start and end filter could be needed. As the function is purely used internally we can still evolve it it if necessary.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
[GitHub] [netbeans] jlahoda commented on a change in pull request #1955:
Hook formating/rangeFormating into LSP Client
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #1955: Hook formating/rangeFormating into LSP Client
URL: https://github.com/apache/netbeans/pull/1955#discussion_r381554130
##########
File path: ide/lsp.client/src/org/netbeans/modules/lsp/client/bindings/Formatter.java
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.
+ */
+package org.netbeans.modules.lsp.client.bindings;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.swing.text.BadLocationException;
+import javax.swing.text.Document;
+import org.eclipse.lsp4j.DocumentFormattingParams;
+import org.eclipse.lsp4j.DocumentRangeFormattingParams;
+import org.eclipse.lsp4j.FormattingOptions;
+import org.eclipse.lsp4j.Range;
+import org.eclipse.lsp4j.TextDocumentIdentifier;
+import org.eclipse.lsp4j.TextEdit;
+import org.netbeans.api.editor.mimelookup.MimeRegistration;
+import org.netbeans.modules.editor.indent.api.IndentUtils;
+import org.netbeans.modules.editor.indent.spi.Context;
+import org.netbeans.modules.editor.indent.spi.ExtraLock;
+import org.netbeans.modules.editor.indent.spi.ReformatTask;
+import org.netbeans.modules.lsp.client.LSPBindings;
+import org.netbeans.modules.lsp.client.Utils;
+import org.openide.filesystems.FileObject;
+import org.openide.loaders.MultiDataObject;
+
+public class Formatter implements ReformatTask {
+
+ private static final Logger LOG = Logger.getLogger(Formatter.class.getName());
+
+ @MimeRegistration(mimeType = "", service = ReformatTask.Factory.class)
+ public static class Factory implements ReformatTask.Factory {
+
+ @Override
+ public ReformatTask createTask(Context context) {
+ return new Formatter(context);
+ }
+
+ }
+
+ private final Context ctx;
+
+ public Formatter(Context ctx) {
+ this.ctx = ctx;
+ }
+
+ @Override
+ public void reformat() throws BadLocationException {
+ Object stream = ctx.document().getProperty(Document.StreamDescriptionProperty);
Review comment:
Better that getting the StreamDescriptionProperty is to use (org.netbeans.modules.editor.)NbEditorUtilities.getFileObject (less code and less error prone). We could also add LSPBindings.getBindings(Document), to simplify the code, but surely not urgent.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
[GitHub] [netbeans] matthiasblaesing merged pull request #1955: Hook
formating/rangeFormating into LSP Client
Posted by GitBox <gi...@apache.org>.
matthiasblaesing merged pull request #1955: Hook formating/rangeFormating into LSP Client
URL: https://github.com/apache/netbeans/pull/1955
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
[GitHub] [netbeans] jlahoda commented on a change in pull request #1955:
Hook formating/rangeFormating into LSP Client
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #1955: Hook formating/rangeFormating into LSP Client
URL: https://github.com/apache/netbeans/pull/1955#discussion_r381558984
##########
File path: ide/lsp.client/src/org/netbeans/modules/lsp/client/bindings/Formatter.java
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.
+ */
+package org.netbeans.modules.lsp.client.bindings;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.swing.text.BadLocationException;
+import javax.swing.text.Document;
+import org.eclipse.lsp4j.DocumentFormattingParams;
+import org.eclipse.lsp4j.DocumentRangeFormattingParams;
+import org.eclipse.lsp4j.FormattingOptions;
+import org.eclipse.lsp4j.Range;
+import org.eclipse.lsp4j.TextDocumentIdentifier;
+import org.eclipse.lsp4j.TextEdit;
+import org.netbeans.api.editor.mimelookup.MimeRegistration;
+import org.netbeans.modules.editor.indent.api.IndentUtils;
+import org.netbeans.modules.editor.indent.spi.Context;
+import org.netbeans.modules.editor.indent.spi.ExtraLock;
+import org.netbeans.modules.editor.indent.spi.ReformatTask;
+import org.netbeans.modules.lsp.client.LSPBindings;
+import org.netbeans.modules.lsp.client.Utils;
+import org.openide.filesystems.FileObject;
+import org.openide.loaders.MultiDataObject;
+
+public class Formatter implements ReformatTask {
+
+ private static final Logger LOG = Logger.getLogger(Formatter.class.getName());
+
+ @MimeRegistration(mimeType = "", service = ReformatTask.Factory.class)
+ public static class Factory implements ReformatTask.Factory {
+
+ @Override
+ public ReformatTask createTask(Context context) {
+ return new Formatter(context);
+ }
+
+ }
+
+ private final Context ctx;
+
+ public Formatter(Context ctx) {
+ this.ctx = ctx;
+ }
+
+ @Override
+ public void reformat() throws BadLocationException {
+ Object stream = ctx.document().getProperty(Document.StreamDescriptionProperty);
+ if(stream instanceof MultiDataObject) {
+ MultiDataObject mdo = ((MultiDataObject) stream);
+ FileObject fo = mdo.getPrimaryFile();
+ LSPBindings bindings = LSPBindings.getBindings(fo);
+ if (bindings != null) {
+ Boolean documentFormatting = bindings.getInitResult().getCapabilities().getDocumentFormattingProvider();
+ Boolean rangeFormatting = bindings.getInitResult().getCapabilities().getDocumentRangeFormattingProvider();
+ if (rangeFormatting != null && rangeFormatting) {
+ rangeFormat(fo, bindings);
+ } else if (documentFormatting != null && documentFormatting) {
+ documentFormat(fo, bindings);
+ }
+ }
+ }
+ }
+
+ private void rangeFormat(FileObject fo, LSPBindings bindings) throws BadLocationException {
+ DocumentRangeFormattingParams drfp = new DocumentRangeFormattingParams();
+ drfp.setTextDocument(new TextDocumentIdentifier(Utils.toURI(fo)));
+ drfp.setOptions(new FormattingOptions(
+ IndentUtils.indentLevelSize(ctx.document()),
+ IndentUtils.isExpandTabs(ctx.document())));
+ drfp.setRange(new Range(
+ Utils.createPosition(ctx.document(), ctx.startOffset()),
+ Utils.createPosition(ctx.document(), ctx.endOffset())));
+ List<? extends TextEdit> edits = Collections.EMPTY_LIST;
+ try {
+ edits = new ArrayList<>(bindings.getTextDocumentService().rangeFormatting(drfp).get());
+ } catch (InterruptedException | ExecutionException ex) {
+ LOG.log(Level.INFO,
+ String.format("LSP document rangeFormat failed for {0}", fo),
+ ex);
+ }
+ Collections.sort(edits, rangeReverseSort);
+
+ applyTextEdits(edits);
+ }
+
+ private void documentFormat(FileObject fo, LSPBindings bindings) throws BadLocationException {
+ DocumentFormattingParams dfp = new DocumentFormattingParams();
+ dfp.setTextDocument(new TextDocumentIdentifier(Utils.toURI(fo)));
+ dfp.setOptions(new FormattingOptions(
+ IndentUtils.indentLevelSize(ctx.document()),
+ IndentUtils.isExpandTabs(ctx.document())));
+ List<? extends TextEdit> edits = Collections.EMPTY_LIST;
+ try {
+ edits = new ArrayList<>(bindings.getTextDocumentService().formatting(dfp).get());
+ } catch (InterruptedException | ExecutionException ex) {
+ LOG.log(Level.INFO,
+ String.format("LSP document format failed for {0}", fo),
+ ex);
+ }
+ applyTextEdits(edits);
+ }
+
+ private void applyTextEdits(List<? extends TextEdit> edits) throws BadLocationException {
+ Collections.sort(edits, rangeReverseSort);
Review comment:
Would it make sense to call Utils.applyEditsNoLock here? Could be a new overload taking "start" and "end" offsets; or a predicate like "BiPredicate<Integer, Integer> acceptor", which would then be used to filter out the interesting edits (although the latter feels fairly ugly). Would help in case we find a bug in the way the edits are applied.
(the rangeReverseSort is a nicer variant of what is in Utils.applyEditsNoLock, so could be moved into Utils and be used instead of the current code.)
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
[GitHub] [netbeans] jlahoda commented on a change in pull request #1955:
Hook formating/rangeFormating into LSP Client
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #1955: Hook formating/rangeFormating into LSP Client
URL: https://github.com/apache/netbeans/pull/1955#discussion_r381554698
##########
File path: ide/lsp.client/src/org/netbeans/modules/lsp/client/bindings/Formatter.java
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.
+ */
+package org.netbeans.modules.lsp.client.bindings;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.swing.text.BadLocationException;
+import javax.swing.text.Document;
+import org.eclipse.lsp4j.DocumentFormattingParams;
+import org.eclipse.lsp4j.DocumentRangeFormattingParams;
+import org.eclipse.lsp4j.FormattingOptions;
+import org.eclipse.lsp4j.Range;
+import org.eclipse.lsp4j.TextDocumentIdentifier;
+import org.eclipse.lsp4j.TextEdit;
+import org.netbeans.api.editor.mimelookup.MimeRegistration;
+import org.netbeans.modules.editor.indent.api.IndentUtils;
+import org.netbeans.modules.editor.indent.spi.Context;
+import org.netbeans.modules.editor.indent.spi.ExtraLock;
+import org.netbeans.modules.editor.indent.spi.ReformatTask;
+import org.netbeans.modules.lsp.client.LSPBindings;
+import org.netbeans.modules.lsp.client.Utils;
+import org.openide.filesystems.FileObject;
+import org.openide.loaders.MultiDataObject;
+
+public class Formatter implements ReformatTask {
+
+ private static final Logger LOG = Logger.getLogger(Formatter.class.getName());
+
+ @MimeRegistration(mimeType = "", service = ReformatTask.Factory.class)
+ public static class Factory implements ReformatTask.Factory {
+
+ @Override
+ public ReformatTask createTask(Context context) {
+ return new Formatter(context);
+ }
+
+ }
+
+ private final Context ctx;
+
+ public Formatter(Context ctx) {
+ this.ctx = ctx;
+ }
+
+ @Override
+ public void reformat() throws BadLocationException {
+ Object stream = ctx.document().getProperty(Document.StreamDescriptionProperty);
+ if(stream instanceof MultiDataObject) {
+ MultiDataObject mdo = ((MultiDataObject) stream);
+ FileObject fo = mdo.getPrimaryFile();
+ LSPBindings bindings = LSPBindings.getBindings(fo);
+ if (bindings != null) {
+ Boolean documentFormatting = bindings.getInitResult().getCapabilities().getDocumentFormattingProvider();
+ Boolean rangeFormatting = bindings.getInitResult().getCapabilities().getDocumentRangeFormattingProvider();
+ if (rangeFormatting != null && rangeFormatting) {
+ rangeFormat(fo, bindings);
+ } else if (documentFormatting != null && documentFormatting) {
+ documentFormat(fo, bindings);
+ }
+ }
+ }
+ }
+
+ private void rangeFormat(FileObject fo, LSPBindings bindings) throws BadLocationException {
+ DocumentRangeFormattingParams drfp = new DocumentRangeFormattingParams();
+ drfp.setTextDocument(new TextDocumentIdentifier(Utils.toURI(fo)));
+ drfp.setOptions(new FormattingOptions(
+ IndentUtils.indentLevelSize(ctx.document()),
+ IndentUtils.isExpandTabs(ctx.document())));
+ drfp.setRange(new Range(
+ Utils.createPosition(ctx.document(), ctx.startOffset()),
+ Utils.createPosition(ctx.document(), ctx.endOffset())));
+ List<? extends TextEdit> edits = Collections.EMPTY_LIST;
Review comment:
Nit: might be better to use Collections.emptyList(), as using Collections.EMPTY_LIST leads to an unchecked cast (at least I believe so).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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