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

[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