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 2022/08/30 04:55:38 UTC

[GitHub] [netbeans] lkishalmi opened a new pull request, #4565: TOML Cleanup and Improvements

lkishalmi opened a new pull request, #4565:
URL: https://github.com/apache/netbeans/pull/4565

   Well, this one got a bit fatter I've initially planned.
   
   It has the same cleanup applied for Yaml and Dockerfile support.
   
   Additionally I've found the root cause of the IndexOutOfBoundException as the Lexer state was not fully saved and restored. Had to do an ugly reflection hack for that, but seems to work.
   
   As a second commit I added the parser Syntax Error output to the UI.


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #4565: TOML Cleanup and Improvements

Posted by GitBox <gi...@apache.org>.
lkishalmi commented on code in PR #4565:
URL: https://github.com/apache/netbeans/pull/4565#discussion_r959787106


##########
ide/languages.toml/src/org/netbeans/modules/languages/toml/LexerInputCharStream.java:
##########
@@ -55,51 +56,62 @@ public int LA(int count) {
         int c = 0;
         if (count > 0) {
             for (int i = 0; i < count; i++) {
-                c = input.read();
+                c = read();
             }
-            input.backup(count);
+            backup(count);
         } else {
-            input.backup(count);
-            c = input.read();
+            backup(count);
+            c = read();
         }
         return c;
     }
 
+    //Marks are for buffering in ANTLR4, we do not really need them

Review Comment:
   Well, throwing out unmarked sections of StringBuilder could be implemented one day with mark supported.
   
   Looked around the Lexing API yesterday. Accessing the underlying buffers is not as easy as I've thought.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #4565: TOML Cleanup and Improvements

Posted by GitBox <gi...@apache.org>.
lkishalmi commented on code in PR #4565:
URL: https://github.com/apache/netbeans/pull/4565#discussion_r959046290


##########
ide/languages.toml/src/org/netbeans/modules/languages/toml/LexerInputCharStream.java:
##########
@@ -55,51 +56,62 @@ public int LA(int count) {
         int c = 0;
         if (count > 0) {
             for (int i = 0; i < count; i++) {
-                c = input.read();
+                c = read();
             }
-            input.backup(count);
+            backup(count);
         } else {
-            input.backup(count);
-            c = input.read();
+            backup(count);
+            c = read();
         }
         return c;
     }
 
+    //Marks are for buffering in ANTLR4, we do not really need them

Review Comment:
   LexerInput ```readText```, ```readLength```  is scoped for the actual token being processed. CharStream ```getIndex``` and ```getText``` are work on stream level. Also ```getText``` is kind of an optional method. Fortunately it used in all Lexers. Just discovered to have a problem with the previos implementation on the Antlr lexer.
   
   I'm tempted to look around the Lexing API and probably add a more ANTLR friendly interface. Will, see. I do not think that this would be the final implementation. It is kind of good enough for now.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] matthiasblaesing commented on a diff in pull request #4565: TOML Cleanup and Improvements

Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on code in PR #4565:
URL: https://github.com/apache/netbeans/pull/4565#discussion_r958417629


##########
ide/languages.toml/src/org/netbeans/modules/languages/toml/TomlLanguage.java:
##########
@@ -19,14 +19,90 @@
 package org.netbeans.modules.languages.toml;
 
 import org.netbeans.api.lexer.Language;
+import org.netbeans.core.spi.multiview.MultiViewElement;
+import org.netbeans.core.spi.multiview.text.MultiViewEditorElement;
 import org.netbeans.modules.csl.spi.DefaultLanguageConfig;
 import org.netbeans.modules.csl.spi.LanguageRegistration;
+import org.netbeans.modules.parsing.spi.Parser;
+import org.openide.awt.ActionID;
+import org.openide.awt.ActionReference;
+import org.openide.awt.ActionReferences;
+import org.openide.filesystems.MIMEResolver;
+import org.openide.util.Lookup;
+import org.openide.util.NbBundle;
+import org.openide.windows.TopComponent;
 
 /**
  *
  * @author lkishalmi
  */
-@LanguageRegistration(mimeType = TomlTokenId.TOML_MIME_TYPE) //NOI18N
+@NbBundle.Messages(
+        "TOMLResolver=Toml File"
+)
+@MIMEResolver.ExtensionRegistration(displayName = "#TOMLResolver",
+    extension = "toml",
+    mimeType = TomlTokenId.TOML_MIME_TYPE,
+    position = 285
+)
+
+@ActionReferences({
+    @ActionReference(
+            path = "Loaders/text/x-toml/Actions",
+            id = @ActionID(category = "System", id = "org.openide.actions.OpenAction"),
+            position = 100,
+            separatorAfter = 200
+    ),
+    @ActionReference(
+            path = "Loaders/text/x-toml/Actions",
+            id = @ActionID(category = "Edit", id = "org.openide.actions.CutAction"),
+            position = 300
+    ),
+    @ActionReference(
+            path = "Loaders/text/x-toml/Actions",
+            id = @ActionID(category = "Edit", id = "org.openide.actions.CopyAction"),
+            position = 400
+    ),
+    @ActionReference(
+            path = "Loaders/text/x-toml/Actions",
+            id = @ActionID(category = "Edit", id = "org.openide.actions.PushAction"),

Review Comment:
   Typo?
   
   ```suggestion
               id = @ActionID(category = "Edit", id = "org.openide.actions.PasteAction"),
   ```



##########
ide/languages.toml/src/org/netbeans/modules/languages/toml/TomlLexer.java:
##########
@@ -137,21 +141,45 @@ private Token<TomlTokenId> token(TomlTokenId id) {
     }
 
     private static class LexerState {
+        private static Field ARRAY_DEPTH;
+        private static Field ARRAY_DEPTH_STACK;
+
         final int state;
         final int mode;
-        final IntegerList modes;
+        final IntegerStack modes;
+
+        final int arrayDepth;
+        final IntegerStack arrayDepthStack;
+
+        static {
+            try {
+                // Hack accessing private state parts of TomlLexer
+                // See: https://github.com/tomlj/tomlj/pull/42
+                ARRAY_DEPTH = org.tomlj.internal.TomlLexer.class.getDeclaredField("arrayDepth");
+                ARRAY_DEPTH.setAccessible(true);
+                ARRAY_DEPTH_STACK = org.tomlj.internal.TomlLexer.class.getDeclaredField("arrayDepthStack");
+                ARRAY_DEPTH_STACK.setAccessible(true);
+            } catch (ReflectiveOperationException ex) {
+            }
+        }
 
-        LexerState(org.tomlj.internal.TomlLexer lexer) {
+        LexerState(org.tomlj.internal.TomlLexer lexer) throws IllegalArgumentException, IllegalAccessException {

Review Comment:
   Using reflection is an implementation detail here - I would catch IllegalAccessException and wrap it into a RuntimeException. In the future you hopefully have access to the lexer state and thus the exceptions will not be thrown.



##########
ide/languages.toml/src/org/netbeans/modules/languages/toml/LexerInputCharStream.java:
##########
@@ -55,51 +56,62 @@ public int LA(int count) {
         int c = 0;
         if (count > 0) {
             for (int i = 0; i < count; i++) {
-                c = input.read();
+                c = read();
             }
-            input.backup(count);
+            backup(count);
         } else {
-            input.backup(count);
-            c = input.read();
+            backup(count);
+            c = read();
         }
         return c;
     }
 
+    //Marks are for buffering in ANTLR4, we do not really need them

Review Comment:
   If I read the documentation correctly, we actually need buffering/seekability of the consumed stream (the documentation of CharStream declares this as required for lookahead), but the LexerInput already handles that - isn't it?
   
   Reading further you actually buffer the whole file in memory, which kind of defeats the purpose of the `CharStream`  implementation. `getText` can only look back to places, that are protected by a mark, so the buffer is limited (assuming limited lookahead and marking).



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] lkishalmi merged pull request #4565: TOML Cleanup and Improvements

Posted by GitBox <gi...@apache.org>.
lkishalmi merged PR #4565:
URL: https://github.com/apache/netbeans/pull/4565


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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


[GitHub] [netbeans] matthiasblaesing commented on a diff in pull request #4565: TOML Cleanup and Improvements

Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on code in PR #4565:
URL: https://github.com/apache/netbeans/pull/4565#discussion_r959708265


##########
ide/languages.toml/src/org/netbeans/modules/languages/toml/LexerInputCharStream.java:
##########
@@ -55,51 +56,62 @@ public int LA(int count) {
         int c = 0;
         if (count > 0) {
             for (int i = 0; i < count; i++) {
-                c = input.read();
+                c = read();
             }
-            input.backup(count);
+            backup(count);
         } else {
-            input.backup(count);
-            c = input.read();
+            backup(count);
+            c = read();
         }
         return c;
     }
 
+    //Marks are for buffering in ANTLR4, we do not really need them

Review Comment:
   Seeing, that ANTLR does not bother to implement its own interface (`ANTLRInputStream`) in an efficient way, I can't argue, that this implementation is inefficient, so ignore that.
   For the tempation to change the lexer API to be "ANTLR" friedly: before going there, make sure you have a very good reason, at some point ANTLR will go away and we will retain the fallout.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

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