[GitHub] [daffodil] tuxji commented on a change in pull request #514: Update jansi to 2.3.2

[GitBox <gi...@apache.org>]

tuxji commented on a change in pull request #514:
URL: https://github.com/apache/daffodil/pull/514#discussion_r607914250



##########
File path: daffodil-cli/src/it/scala/org/apache/daffodil/debugger/TestCLIDebugger.scala
##########
@@ -28,11 +28,9 @@ import org.apache.daffodil.CLI.Util
 
 class TestCLIdebugger {
 
-  val DAFFODIL_JAVA_OPTS = Map("DAFFODIL_JAVA_OPTS" -> "-Xms256m -Xmx2048m -Djline.terminal=jline.UnsupportedTerminal -Dfile.encoding=UTF-8")
-  //  Dubugging tests were not executing under Windows and especially under Eclipse
-  //  due to the use of a non-interactive console.
+  val DAFFODIL_JAVA_OPTS = Map("DAFFODIL_JAVA_OPTS" -> "-Xms256m -Xmx2048m -Dfile.encoding=UTF-8")
   //  Set the DAFFODIL_JAVA_OPTS environment variable for Debugger tests to specify
-  //  the use of an unsupported terminal: -Djline.terminal=jline.UnsupportedTerminal
+  //  the use of a dumb terminal: -Dorg.jline.terminal.dumb

Review comment:
       No, because I had a hunch that JLine 3 wouldn't need -Dorg.jline.terminal.dumb in DAFFODIL_JAVA_OPTS anyway.  I noticed from my scanning of the JLine 3 sources that JLine 3 goes to some effort to pick the correct terminal implementation (including the dumb terminal) using the TERM environment variable and so on.  I updated the comment to show how to specify the use of a dumb terminal in case we ever need to do it in the future, but we didn't need it to make the Debugger tests pass (although it's also true that the Debugger tests hit only a third of the JLine 3 code paths according to the coverage report).

##########
File path: daffodil-runtime1/src/main/scala/org/apache/daffodil/debugger/InteractiveDebugger.scala
##########
@@ -555,20 +558,15 @@ class InteractiveDebugger(runner: InteractiveDebuggerRunner, eCompilers: Express
         if (args != "") {
           if (dc == cmd) {
             val completeString = getCompleteString(args)
-            val subcandidates = new java.util.ArrayList[CharSequence]
-            val newCursor = subcommandsCompleter.complete(completeString, cursor, subcandidates)
+            val subline = reader.getParser.parse(completeString, 0)
+            val subcandidates = new java.util.ArrayList[Candidate]
+            subcommandsCompleter.complete(reader, subline, subcandidates)
             val seq = subcandidates.asScala
             seq.foreach(c => candidates.add(c))
-            buffer.lastIndexOf(completeString) + newCursor
-          } else {
-            -1
           }
         } else {
           if (dc.name.startsWith(cmd)) {
-            candidates.add(dc.name + " ")
-            buffer.lastIndexOf(cmd)
-          } else {
-            -1
+            candidates.add(new Candidate(dc.name + " "))

Review comment:
       Yes, we should remove that extra space.  You're probably right that the original author of that code added the space to work around some issue with autocomplete that has since been fixed.  I suspect that the subcandidate completion code below is causing the issue you noticed with main command completions sneaking into subcommands too.




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