Skip to content

JLine 3: tab completion: showing method signatures is broken #696

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 · 8 comments · Fixed by scala/scala#8848
Closed

JLine 3: tab completion: showing method signatures is broken #696

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

Comments

@SethTisue
Copy link
Member

SethTisue commented Mar 7, 2020

as I said on the PR

the behavior when you hit the TAB key more than once is a regression. It seems like a blocker to me

if you tab twice, instead of showing you the signature in an information area, it actually inserts it into your command line. here I typed Predef.ident and then double-tabbed

scala> Predef.def identity[A](x: A): A

whereas on JLine 2, the interaction looks as follows. it shows the signature and then gives you a whole new prompt:

scala 2.13.1> Predef.identity
   def identity[A](x: A): A

scala 2.13.1> Predef.identity

Ammonite behaves similarly (even though they're on JLine 3):

@ Predef.iden 
identity
@ Predef.identity 
def identity[A](x: A): A
@ Predef.identity

whereas Dotty doesn't seem to have this signature-showing feature

anyway, we need to either bring that behavior back or find another way to show this information, perhaps using JLine's "status" feature

scala/scala#7645 (comment) : "It's nice that autocomplete doesn't burn lines of console anymore, the options are updated in-place on subsequent typing. now takes you into the matrix of completions to navigate with a cursor. Previously, we'd detect and emit the signature for the first candidate. I'm sure JLine3 has a better way to render this, perhaps in a status bar."

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

SethTisue commented Mar 26, 2020

@Jasper-M
Copy link

However this gets fixed, I would do the same thing for #692

@SethTisue SethTisue added this to the 2.13.2 milestone Mar 31, 2020
@SethTisue
Copy link
Member Author

SethTisue commented Apr 2, 2020

here's what Ammonite is doing:

    // display method signature(s)
    if (sigs.nonEmpty) {
      reader.getTerminal.writer.println()
      sigs.foreach{ sig =>
        val sigHighlighted = highlighter.highlight(reader, sig).toAnsi
        reader.getTerminal.writer.println(sigHighlighted)
      }
      reader.callWidget(LineReader.REDRAW_LINE)
      reader.callWidget(LineReader.REDISPLAY)
      reader.getTerminal.flush()
    }

@SethTisue
Copy link
Member Author

if we go with the JLine 3 "status" API, using it appears to be as simple as:

Status status = Status.getStatus(reader.getTerminal());
status.update(<String>);

as for where the signature information is coming from, our code calls the signatures defStrings.

some refactoring may be called for here, as our PresentationCompilation class only provides a single candidates method, which then internally decides whether to return the result of its internal defStringCandidates, or return a normal set of candidates

if we don't refactor that, then we'd need to inspect the candidate set to see which kind it is. this may be possible actually, as the defString "candidates" are returned in a List[String] with the empty string in head as a sentinel:

    type Candidates = (Int, List[String])
    override def candidates(tabCount: Int): Candidates = {
      ...
      def defStringCandidates(matching: List[Member], name: Name, isNew: Boolean): Candidates = {
        ...
        (cursor, "" :: defStrings.distinct)
      }

@SethTisue
Copy link
Member Author

I pushed some messy code for trying out the "status" feature for this to https://github.com/SethTisue/scala/tree/defstring-in-status

it sort of works a little, but not really... the status appears, but then the cursor ends up stuck at the beginning of the line

rather than mess with it for 2.13.2, I think it's probably better to do it in the Ammonite style for now, since doing it that way should be foolproof and should get us to a shippable release — and then leave a possible re-do using the status feature as future work

@SethTisue
Copy link
Member Author

SethTisue commented Apr 3, 2020

the defString "candidates" are returned in a List[String] with the empty string in head as a sentinel

Oh, I think I understand why it was done this way.

The way the old JLine 2 based code presented method signatures was by hijacking the UI for presenting completion options. Method signatures were presented as if they were normal completions. But if there's only one completion available, it would automatically complete. In order for the completions to merely be displayed, there had to be at least two completions, so the empty string was thrown in as one of them.

But — and now it gets more speculative — I guess that JLine 3 filters out the extra empty string and concludes that only a single completion is available, which is why the signature gets inserted into the input buffer?

SethTisue added a commit to SethTisue/scala that referenced this issue Apr 3, 2020
@SethTisue
Copy link
Member Author

I've added a commit to scala/scala#8848 that handles this at least reasonably okay

@Jasper-M
Copy link

Jasper-M commented Apr 3, 2020

But — and now it gets more speculative — I guess that JLine 3 filters out the extra empty string and concludes that only a single completion is available, which is why the signature gets inserted into the input buffer?

It's more complicated than just filtering out empty strings cause for the //print thing I tried returning "foo" :: actualPrintThing() :: "bar" :: Nil and JLine still completely ignored the foo and bar suggestions...

SethTisue added a commit to SethTisue/scala that referenced this issue Apr 6, 2020
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.

2 participants