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

Add workspace/executeCommand and clearIndexCache command #131

Merged
merged 3 commits into from
Dec 15, 2017

Conversation

gabro
Copy link
Member

@gabro gabro commented Dec 15, 2017

Closes #109

@@ -323,6 +343,18 @@ object ScalametaLanguageServer extends LazyLogging {
path
}

def clearCacheDirectory(): Unit =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if somebody knows a better way of recursively deleting a directory with Java NIO, please let me know!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the best way to do it IMO :) It should be verbose!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a more serious note, this might merit inclusion in org.langmeta.io.FileIO where we have similar utils

WorkspaceCommand.withNameOption(request.params.command).map {
case ClearIndexCache =>
logger.info("Clearing the index cache")
ScalametaLanguageServer.clearCacheDirectory()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we currently clear the index, but we don't trigger re-indexing. I think it's fine for debugging to have this level of granularity, but we may want to add a separate command for triggering re-indexing.

import enumeratum.EnumEntry
import enumeratum.EnumEntry.Uncapitalised

sealed trait WorkspaceCommand extends EnumEntry with Uncapitalised
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went for a lowerCamelCase convention here and thanks to Uncapitalised this is automatically handled by withNameOption and entryName

case ClearIndexCache =>
logger.info("Clearing the index cache")
ScalametaLanguageServer.clearCacheDirectory()
}.getOrElse(logger.error(s"Unknown command ${request.params.command}"))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the server declares which messages it responds to, but that doesn't prevent the VSCode client to send arbitrary messages, so we log an error to help debugging typos and other mistakes.

@gabro gabro requested a review from olafurpg December 15, 2017 11:19
@@ -317,6 +317,12 @@ object DocumentFormattingParams {
implicit val format = Json.format[DocumentFormattingParams]
}

case class WorkspaceExecuteCommandParams(command: String, arguments: Option[Seq[JsValue]])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arguments is any[] in the LSP, so I figured JsValue is a good type is.

Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet! I will follow up with a command to askReset() the compiler instances. LGTM 👍

@@ -323,6 +343,18 @@ object ScalametaLanguageServer extends LazyLogging {
path
}

def clearCacheDirectory(): Unit =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the best way to do it IMO :) It should be verbose!

@@ -323,6 +343,18 @@ object ScalametaLanguageServer extends LazyLogging {
path
}

def clearCacheDirectory(): Unit =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a more serious note, this might merit inclusion in org.langmeta.io.FileIO where we have similar utils

@@ -91,5 +92,12 @@ export async function activate(context: ExtensionContext) {
}
});

client.onReady().then(() => {
const clearIndexCacheCommand = commands.registerCommand("scalameta.clearIndexCache", async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for every new command? I thought codeActions was a way for the server to list the commands it supports.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is. codeAction is not very well integrated in the client library, as of now. I guess it may depend by the requirement for extensions to statically include commands in the manifest

@gabro gabro merged commit 892e5f1 into scalameta:master Dec 15, 2017
@gabro gabro deleted the workspace-execute-command branch December 15, 2017 11:52
@ShaneDelmore
Copy link
Collaborator

@gabro For future reference if you prefer you can use the java streams api. I’m not at my computer but it would be something like:

Files.walk(pathToDelete) //You need to follow links, I think that is default behavior, can’t remember
.sorted(Comparator.reverseOrder())
.map(_.toFile)
.forEach(File.delete);

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