Skip to content

JLine 3: resolve double-tab behavior somehow for 2.13.2 #698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SethTisue opened this issue Mar 7, 2020 · 7 comments · Fixed by scala/scala#8905
Closed

JLine 3: resolve double-tab behavior somehow for 2.13.2 #698

SethTisue opened this issue Mar 7, 2020 · 7 comments · Fixed by scala/scala#8905
Assignees
Labels
blocker t:repl JLine 3 upgrade (scala/scala#8036)
Milestone

Comments

@SethTisue
Copy link
Member

under JLine 2 double tab had this behavior: on first tab it only shows the methods you're likeliest to want, then on second tab it adds more: things like toString, and also methods you only get through implicit conversions:

scala 2.13.1> class C { def x = 3; def y = 4 }
defined class C

scala 2.13.1> (new C).
x   y

scala 2.13.1> (new C).
!=   ->             ensuring   formatted   isInstanceOf   notifyAll      wait   →   
##   ==             eq         getClass    ne             synchronized   x          
+    asInstanceOf   equals     hashCode    notify         toString       y          

note the presence of ensuring, which is an extension method.

under JLine 3, that behavior still exists, but in a funny way. you have to ask for the same completion again, and that gives you what used to be the double-tab behavior.

ah, JLine 3 has a "grouping" feature, where you can divide offered completions into groups, and even show a name for each group! so we could have a main group, an extension method group, and an "other" group. maybe a "deprecated" group (scala/scala#7869)

even if we can't get the grouping thing going in time, we need to do something for 2.13.2, IMO

@SethTisue SethTisue added t:repl JLine 3 upgrade (scala/scala#8036) blocker labels Mar 7, 2020
@SethTisue SethTisue added this to the 2.13.2 milestone Mar 31, 2020
@SethTisue
Copy link
Member Author

as I already noted on scala/scala#7869, an alternative to using the grouping feature would be

to set the descr field of org.jline.reader.Candidate to "deprecated". this would show up in the UI as e.g.:

Screen Shot 2020-04-02 at 6 21 03 PM

@SethTisue
Copy link
Member Author

SethTisue commented Apr 14, 2020

as for how to make this happen in the code,

PresentationCompilationResult has type Candidates = (Int, List[String]) and def candidates(tabCount: Int): Candidates

This is compiler stuff so we're free to change these signatures, but doing so makes the tooling authors grumpy. A minimalist, good-enough-for-2.13.2 approach might be to leave all of that alone for now and call candidates twice, once with tabCount of 0 and again with tabCount of 1, and then present the results in two groups.

But it's kind of a craptastic solution, because there are multiple reasons something might not show up until the second tab: there's deprecation, but there's also stuff like ## being filtered out until the second tab; we can't distinguish between those without changing the API.

I suppose we could continue to offer the old API but deprecate it, and have better API alongside it.

@SethTisue SethTisue self-assigned this Apr 15, 2020
@SethTisue
Copy link
Member Author

SethTisue commented Apr 15, 2020

enabling groups is simple, as shown below, but I have a major frustrations with the resulting behavior: keyboard navigation is all messed up, the arrow keys make the cursor jump around weirdly

so in order to go the groups route, we'd either need to figure out if there's a workaround, or submit a fix to the JLine folks upstream

--- src/repl-frontend/scala/tools/nsc/interpreter/jline/Reader.scala
+++ src/repl-frontend/scala/tools/nsc/interpreter/jline/Reader.scala
@@ -85,12 +85,13 @@ object Reader {
     locally {
       import LineReader._, Option._
       builder
-        .option(AUTO_GROUP, false)
+        .option(AUTO_GROUP, true)
         .option(LIST_PACKED, true)  // TODO
         .option(INSERT_TAB, true)   // At the beginning of the line, insert tab instead of completing
         .variable(HISTORY_FILE, config.historyFile) // Save history to file
         .variable(SECONDARY_PROMPT_PATTERN, config.encolor(config.continueText)) // Continue prompt
-        .option(Option.DISABLE_EVENT_EXPANSION, true) // Otherwise `scala> println(raw"\n".toList)` gives `List(n)` !!
+        .variable(OTHERS_GROUP_NAME, " ")
+        .option(DISABLE_EVENT_EXPANSION, true) // Otherwise `scala> println(raw"\n".toList)` gives `List(n)` !!
     }
 
     val reader = builder.build()
@@ -231,11 +232,11 @@ class Completion(delegate: shell.Completion) extends shell.Completion with Compl
         case CompletionCandidate.Nilary => "()"
         case _ => "("
       })
-      val group = null        // results may be grouped
-      val descr =             // displayed alongside
+      val group =             // resulted may be grouped
         if (cc.isDeprecated) "deprecated"
         else if (cc.isUniversal) "universal"
         else null
+      val descr = null        // displayed alongside
       val suffix = null       // such as slash after directory name
       val key = null          // same key implies mergeable result
       val complete = false    // more to complete?

@SethTisue
Copy link
Member Author

SethTisue commented Apr 15, 2020

set the descr field

I have working code that does this, but in the absence of grouping, it's pretty annoying to have all the methods mixed together, so I guess the next step is to do the grouping ourselves by sorting

@SethTisue
Copy link
Member Author

SethTisue commented Apr 22, 2020

I guess the next step is to do the grouping ourselves by sorting

FAIL — JLine wants to sort the candidates itself (by value, which is the actual string that will be inserted when a completion is selected), and I don't see any way to override this. at least, I sunk as much time into it as I'm willing to sink at the moment

for 2.13.2, we'll have to live with everything mixed together:

Screen Shot 2020-04-21 at 8 30 21 PM

which is not so bad, really, given the presence of the (deprecated) and (universal) markers

@SethTisue
Copy link
Member Author

closing on the expectation scala/scala#8905 will be merged

@SethTisue
Copy link
Member Author

this seems to indicate that we should try the grouping feature again in JLine 3.16.1 once it is released: jline/jline3#580

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker t:repl JLine 3 upgrade (scala/scala#8036)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant