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

Change displayBuildTarget to listBuildTargets #3649

Merged
merged 1 commit into from
Feb 21, 2022

Conversation

Arthurm1
Copy link
Contributor

This removes the target-info-display command and adds the list-build-targets command

list-build-targets will return a list of build target names to the client.

It's then up to the client to prompt the user with those targets and send the file-decode command to Metals with the appropriate uri (e.g. metalsDecode:file:///workspacePath/buildTargetName.metals-buildtarget)

@Arthurm1
Copy link
Contributor Author

@ckipp01 Is this more what you had in mind? See the linked VSCode PR for implementation on the client side.

Note it also has to deal with transforming the project URIs in the MetalsTreeView - not sure if VIM implements a TreeView?
They're of the format...
projects:file:/root/metals/.mtags/?id=mtags3!/_root_/,
and need to be in...
metalsDecode:file:///root/metals/mtags3.metals-buildtarget
The build target name is in the id= section

@ckipp01
Copy link
Member

ckipp01 commented Feb 18, 2022

@ckipp01 Is this more what you had in mind? See the linked VSCode PR for implementation on the client side.

Works great for me @Arthurm1, thanks a lot for going back and address this.

2022-02-18 19 08 13

Note it also has to deal with transforming the project URIs in the MetalsTreeView - not sure if VIM implements a TreeView?

No the tree view that is in nvim-metals is very minimal so it doesn't implement this at all.

ckipp01 added a commit to ckipp01/nvim-metals that referenced this pull request Feb 18, 2022
This uses the new `list-build-targets` that is going to be introduced in
scalameta/metals#3649. This might change a bit
yet, but this is roughly what will be needed to use this.
@Arthurm1
Copy link
Contributor Author

I don't think the failing tests are relevant...

tests.pc.InsertInferredTypeSuite
tests.hover.HoverScala3TypeSuite

@tgodzik
Copy link
Contributor

tgodzik commented Feb 21, 2022

Thanks again for changing it! 🎉

@tgodzik tgodzik merged commit a7c90f9 into scalameta:main Feb 21, 2022
ckipp01 added a commit to ckipp01/nvim-metals that referenced this pull request Feb 21, 2022
This uses the new `list-build-targets` that is going to be introduced in
scalameta/metals#3649. This might change a bit
yet, but this is roughly what will be needed to use this.
@Arthurm1 Arthurm1 deleted the list_build_targets branch February 22, 2022 16:57
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

Successfully merging this pull request may close these issues.

None yet

3 participants