almond icon indicating copy to clipboard operation
almond copied to clipboard

Adopt metals implementation for symbol inspection

Open kiendang opened this issue 3 years ago • 7 comments

Close #764 Close #183

This is a POC PR for displaying type info and docstrings upon symbol inspection without metabrowse by reusing the implementation from the LSP. This depends on mtags and copies over the relevant code from metals with some minor adjustments.

Demo

Can display type info and docstrings in Jupyter contextual help upon inspection (Shift-Tab)

type

almond_inspect_type_docstrings

method

almond_inspect_method_docstrings

def

almond_inspect_def_def

val

almond_inspect_val

Things that do not work (yet)

  • Can't display info for builtin objects like kernel, repl. I guess we can do this by exposing and adding repl.fullImports to code.
  • Can't display docstrings defined in the current session. Might be able to do this by adding a separate docstrings search implementation for things in the current file.
almond_inspect_now_defined_docstrings

Discuss

This definitely needs polishing and is not ready to be merged yet. Just wanna find out if this is the right direction.

kiendang avatar Mar 25 '21 17:03 kiendang

Thanks for reviewing!

do you think this could be added alongside the current metabrowse support?

Yup I'll put that back in

Also, are all copy-pasted classes from mtags necessary? It seems MetalsSymbolDocumentation, JavadocIndexer, and EmptySymbolDocumentation could be re-used from mtags, rather than being copied here. Isn't it?

I didn't copy paste from mtags, only from metals. So Docstrings, ScaladocIndexer, JavadocIndexer, MetalsSymbolDocumentation and EmptySymbolDocumentation are not in mtags but in metals. I already add mtags as a dependency. Personally I would love to see Docstrings, ScaladocIndexer and JavadocIndexergot moved from metals to mtags too. Looking up docstrings from symbol seems like a useful task that sees usage outside of metals (like here) and would ease the maintenance on almond's side. And according to https://github.com/almond-sh/almond/issues/183#issuecomment-426916796 looks like previously it was the original idea? However I'm not familiar with metals repo. Would it be a good idea to raise an issue there that linked to this?

kiendang avatar Mar 30 '21 17:03 kiendang

Precisely, I think org.scalameta:::mtags lives in the metals repository, and these classes are defined in its sources. So they should already be around.

alexarchambault avatar Mar 30 '21 18:03 alexarchambault

So currently metabrowse depends on mtags 0.8.4 while this PR uses 0.10.0. I have a problem with mtags 0.8.4, the same one that causes metabrowse to not work in #764. Could you bump mtags in metabrowse 0.8.4 -> 0.10.0?

kiendang avatar Apr 01 '21 06:04 kiendang

The only rather stable interface in Metals is PresentationCompiler java interface, would you be able to reuse that to get what you need? We have never actually thought of mtags as a library the people could use, so maybe we can add some additional API and make sure it's MIMAd ? I've seen a couple of instances that this might have been useful especially if we don't want to start the entire Metals serwer.

Alternatively (just shouting out an idea), there were some plans to reuse Almond in Metals itself using the VS Code notebook API, but I am just a bit lost in the this area, so not sure if it would overall help the ecosystem.

TLDR; We can expose a stable API in Metals if needed, we just haven't really thought of it. Anything internal is not safe currently to use.

tgodzik avatar Apr 01 '21 11:04 tgodzik

Thanks for taking a look at this!

The only rather stable interface in Metals is PresentationCompiler java interface, would you be able to reuse that to get what you need?

I ported over the documentation method in StandaloneSymbolSearch here so I guess it's the SymbolSearch interface?

We can expose a stable API in Metals if needed

That would be great. If I'm not wrong originally there were talks about exposing an API to go from symbol to docstrings https://github.com/almond-sh/almond/issues/183#issuecomment-426931034

Alternatively (just shouting out an idea), there were some plans to reuse Almond in Metals itself using the VS Code notebook API, but I am just a bit lost in the this area, so not sure if it would overall help the ecosystem.

I've been using notebooks in VS Code for Python and really like it. Would be nice to have Almond there too. But AFAIK supports for language kernels other than Python in VS Code Notebook API is not mature yet. Like you could write an extension that makes Almond works but it will conflict with the Python one. But as a user this is something I would like to see happen eventually.

kiendang avatar Apr 02 '21 16:04 kiendang

I ported over the documentation method in StandaloneSymbolSearch here so I guess it's the SymbolSearch interface?

From what I see GlobalSymbolIndex is only used here in the PR?

Anyway, what I think we might do:

  • open an issue in Metals explaining what we don't to expose out of mtags/metals
  • move Scala interface from internal to scala.meta with stable methods
  • we keep that one stable (though mtags do not adhere to any binary compat, which might be an issue)

Alternatively it might be better to copy over the code if it's small enough.

I would start with an issue in Metals, where all interested parties can discuss what interfaces they would need. Otherwise, we will forget about this the same as with the previous case :sweat:

tgodzik avatar Apr 08 '21 11:04 tgodzik

Sure. I'll prepare a minimal snippet to make it easier to see what's needed to expose and open an issue over at the metals repo.

kiendang avatar Apr 15 '21 09:04 kiendang

Now part of https://github.com/almond-sh/almond/pull/1110

alexarchambault avatar May 05 '23 14:05 alexarchambault