Skip to content
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

Completions inside of backticks #418

Open
harpocrates opened this issue Jan 27, 2025 · 2 comments
Open

Completions inside of backticks #418

harpocrates opened this issue Jan 27, 2025 · 2 comments

Comments

@harpocrates
Copy link

Is your feature request related to a problem? Please describe.

Metals does a good job of suggesting identifier completions when the prefix for the identifier is not backticked. However, once backticks come into play, the experience degrades a bit.

For example, in the following code snippet, the This is a method name completion might or might not still be present. If the initial completion list was incomplete, then typing up to This is a may trigger VS code to ask for document/textCompletions again, but now the text buffer no longer contains a single valid identifier.

object Test {
  def `This is a method name` = 123
  this.This is a @@ // Cursor at `@@`
}

The solution I'd expect to have for the above is that as a user I might need to write the backticks upfront and then trigger completions inside the backticks. That way, as I type the prefix for my method, I stay in the intermediate state of having a valid preceding identifier:

object Test {
  def `This is a method name` = 123
  this.`This is a @@` // Cursor at `@@`
}

However, in this case, Metals gives no completions (not even when just at this.`@@` or this.`T@@`)

(I'm not sure how widespread this (ab)use of backticks is, but I'm working in codebases where there's a fluent-like DSL that heavily encourages these English sentence form names, so completions there are a bit miserable).

Describe the solution you'd like

Solution I think is coherent:

  • completions should work inside of backticks just the same as outside (just that the prefix might now include characters it couldn't in a non-back ticked case)
  • if you are requesting a completion from inside backticks, Metals should not remove the backticks when you select a completion and, (if the selected completion needs backticks) Metals should also not add a second pair of backticks
  • completions should ideally trigger even on the "empty backticks" case, eg `@@` or myObj.`@@` (with Auto-close backticks scala/vscode-scala-syntax#280, this means that you get put into this completing state just by typing a single `)

I went and implemented this for 2.13.x to see how it felt (harpocrates/metals@98d2ce5, though you'll need a small Scala compiler patch as well - see commit). Here's what it looks like:

Basic completions inside of backicks
Image

Manually re-triggering the completion after a space character (mocking up the case where VS code might do this automatically because the original completions were incomplete)
Image

Once inside backticks, completing a non-backticked identifier still leaves the backticks
Image

All of the above should hold for backticked select's as well
Image

Describe alternatives you've considered

We could continue not showing any completions when the identifier prefix under the cursor is backticked. 😄

Additional context

No response

Search terms

completions backticks

@harpocrates
Copy link
Author

I'm opening a feature request instead of a PR mostly because I wanted to be in agreement on the above behavior being desirable before putting in the work to make the above also work for Scala 3 (though in that case a Scala compiler patch probably won't be needed).

If all of this sounds reasonable, I'll polish up the above, make it work for Scala 3 and open a PR.

@tgodzik
Copy link
Contributor

tgodzik commented Jan 27, 2025

I'm opening a feature request instead of a PR mostly because I wanted to be in agreement on the above behavior being desirable before putting in the work to make the above also work for Scala 3 (though in that case a Scala compiler patch probably won't be needed).

If all of this sounds reasonable, I'll polish up the above, make it work for Scala 3 and open a PR.

I think this looks quite reasonable. As for Scala 3, we actually moved most of the code the compiler. The metals codebase retains the 3.3.3 version, later versions all have the code behind mtags interfaces in it. So might worth to see if it maybe already works in later Scala version. In that case I wouldn't bother fixing it for 3.3.3, since we are already almost at 3.3.5 for LTS.

If it doesn't work, you can for sure fix it here and we can port it to the compiler later on. Fixing it in the compiler is another option, but it's a bigger codebase and harder to set up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants