From 9cb223954d49e1ab821d7c936c08381f45273a7b Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 16 Oct 2023 15:15:25 +0200 Subject: [PATCH] improvement: make new file independent of workspace root --- .../internal/builds/NewProjectProvider.scala | 34 ++--- .../internal/metals/MetalsLspService.scala | 3 +- .../internal/metals/WorkspaceLspService.scala | 10 +- .../metals/newScalaFile/NewFileProvider.scala | 121 +++++++++--------- .../test/scala/tests/NewFileLspSuite.scala | 3 + 5 files changed, 90 insertions(+), 81 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala index 4b49fe7c655..e52098ca8f4 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala @@ -74,7 +74,9 @@ class NewProjectProvider( ) withTemplate .flatMapOption { template => - askForPath(Some(base)).mapOptionInside { path => (template, path) } + NewProjectProvider + .askForPath(Some(base), icons, client) + .mapOptionInside { path => (template, path) } } .flatMapOption { case (template, path) => askForName(nameFromPath(template.id), NewScalaProject.enterName) @@ -212,9 +214,19 @@ class NewProjectProvider( } } - private def askForPath( - from: Option[AbsolutePath] - ): Future[Option[AbsolutePath]] = { + // scala/hello-world.g8 -> hello-world + private def nameFromPath(g8Path: String) = { + g8Path.replaceAll(".*/", "").replace(".g8", "") + } +} + +object NewProjectProvider { + + def askForPath( + from: Option[AbsolutePath], + icons: Icons, + client: MetalsLanguageClient, + )(implicit ex: ExecutionContext): Future[Option[AbsolutePath]] = { def quickPickDir(filename: String) = { MetalsQuickPickItem( @@ -250,11 +262,11 @@ class NewProjectProvider( case path if path.itemId == currentDir.id => Future.successful(from) case path if path.itemId == parentDir.id => - askForPath(from.flatMap(_.parentOpt)) + askForPath(from.flatMap(_.parentOpt), icons, client) case path => from match { case Some(nonRootPath) => - askForPath(Some(nonRootPath.resolve(path.itemId))) + askForPath(Some(nonRootPath.resolve(path.itemId)), icons, client) case None => val newRoot = File .listRoots() @@ -263,20 +275,12 @@ class NewProjectProvider( AbsolutePath(root.toPath()) } .headOption - askForPath(newRoot) + askForPath(newRoot, icons, client) } } } - // scala/hello-world.g8 -> hello-world - private def nameFromPath(g8Path: String) = { - g8Path.replaceAll(".*/", "").replace(".g8", "") - } -} - -object NewProjectProvider { - val custom: MetalsQuickPickItem = MetalsQuickPickItem( id = "custom", label = "Custom", diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index a8554a23cec..68276e7d2da 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -634,11 +634,10 @@ class MetalsLspService( ) private val newFileProvider: NewFileProvider = new NewFileProvider( - folder, languageClient, packageProvider, - focusedDocument, scalaVersionSelector, + clientConfig.icons, ) private val symbolSearch: MetalsSymbolSearch = new MetalsSymbolSearch( diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala index d708fb744d6..def71ee16f9 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -943,7 +943,10 @@ class WorkspaceLspService( ).asJavaObject case ServerCommands.NewScalaFile(args) => - val directoryURI = args.lift(0).flatten + val directoryURI = args + .lift(0) + .flatten + .orElse(focusedDocument.map(_.parent.toURI.toString())) val name = args.lift(1).flatten val fileType = args.lift(2).flatten directoryURI @@ -951,7 +954,10 @@ class WorkspaceLspService( .getOrElse(fallbackService) .createFile(directoryURI, name, fileType, isScala = true) case ServerCommands.NewJavaFile(args) => - val directoryURI = args.lift(0).flatten + val directoryURI = args + .lift(0) + .flatten + .orElse(focusedDocument.map(_.parent.toURI.toString())) val name = args.lift(1).flatten val fileType = args.lift(2).flatten directoryURI diff --git a/metals/src/main/scala/scala/meta/internal/metals/newScalaFile/NewFileProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/newScalaFile/NewFileProvider.scala index 07bfdfa87ce..41fd705ee57 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/newScalaFile/NewFileProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/newScalaFile/NewFileProvider.scala @@ -8,7 +8,9 @@ import scala.concurrent.Future import scala.util.Properties import scala.util.control.NonFatal +import scala.meta.internal.builds.NewProjectProvider import scala.meta.internal.metals.ClientCommands +import scala.meta.internal.metals.Icons import scala.meta.internal.metals.Messages.NewScalaFile import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.PackageProvider @@ -26,11 +28,10 @@ import org.eclipse.lsp4j.MessageType import org.eclipse.lsp4j.Range class NewFileProvider( - workspace: AbsolutePath, client: MetalsLanguageClient, packageProvider: PackageProvider, - focusedDocument: () => Option[AbsolutePath], selector: ScalaVersionSelector, + icons: Icons, )(implicit ec: ExecutionContext ) { @@ -40,44 +41,50 @@ class NewFileProvider( name: Option[String], fileType: Option[String], isScala: Boolean, - ): Future[Unit] = { - val directory = directoryUri - .map { uri => - val path = uri.toString.toAbsolutePath - if (path.isFile) - path.parent - else - path - } - .orElse(focusedDocument().map(_.parent)) + ): Future[Unit] = + for { + optDirectory <- directoryUri + .map { uri => + val path = uri.toString.toAbsolutePath + if (path.isFile) + path.parent + else + path + } + .map(x => Future.successful(Some(x))) + .getOrElse(NewProjectProvider.askForPath(None, icons, client)) + _ <- optDirectory + .map { directory => + val newlyCreatedFile = { + fileType.flatMap(getFromString) match { + case Some(ft) => createFile(directory, ft, name) + case None => + val askForKind = + if (isScala) + askForScalaKind( + ScalaVersions.isScala3Version( + selector.scalaVersionForPath(directory) + ) + ) + else askForJavaKind + askForKind.flatMapOption(createFile(directory, _, name)) + } + } - val newlyCreatedFile = { - fileType.flatMap(getFromString) match { - case Some(ft) => createFile(directory, ft, name) - case None => - val askForKind = - if (isScala) - askForScalaKind( - directory.forall(dir => - ScalaVersions.isScala3Version( - selector.scalaVersionForPath(dir) - ) - ) - ) - else askForJavaKind - askForKind.flatMapOption(createFile(directory, _, name)) - } - } - - newlyCreatedFile.map { - case Some((path, cursorRange)) => - openFile(path, cursorRange) - case None => () - } - } + newlyCreatedFile.map { + case Some((path, cursorRange)) => + openFile(path, cursorRange) + case None => () + } + } + .getOrElse { + scribe.warn("Cannot create file, no directory provided.") + Future.successful(()) + } + } yield () private def createFile( - directory: Option[AbsolutePath], + directory: AbsolutePath, fileType: NewFileType, name: Option[String], ) = { @@ -175,15 +182,15 @@ class NewFileProvider( } private def createClass( - directory: Option[AbsolutePath], + directory: AbsolutePath, name: String, kind: NewFileType, ext: String, ): Future[(AbsolutePath, Range)] = { - val path = directory.getOrElse(workspace).resolve(name + ext) + val path = directory.resolve(name + ext) // name can be actually be "foo/Name", where "foo" is a folder to create val className = Identifier.backtickWrap( - directory.getOrElse(workspace).resolve(name).filename + directory.resolve(name).filename ) val template = kind match { case CaseClass => caseClassTemplate(className) @@ -201,10 +208,10 @@ class NewFileProvider( } private def createEmptyFileWithPackage( - directory: Option[AbsolutePath], + directory: AbsolutePath, name: String, ): Future[(AbsolutePath, Range)] = { - val path = directory.getOrElse(workspace).resolve(name + ".scala") + val path = directory.resolve(name + ".scala") val pkg = packageProvider .packageStatement(path) .map(_.fileContent) @@ -215,33 +222,23 @@ class NewFileProvider( } private def createPackageObject( - directory: Option[AbsolutePath] + directory: AbsolutePath ): Future[(AbsolutePath, Range)] = { - directory - .map { directory => - val path = directory.resolve("package.scala") - createFileAndWriteText( - path, - packageProvider - .packageStatement(path) - .getOrElse(NewFileTemplate.empty), - ) - } - .getOrElse( - Future.failed( - new IllegalArgumentException( - "'directory' must be provided to create a package object" - ) - ) - ) + val path = directory.resolve("package.scala") + createFileAndWriteText( + path, + packageProvider + .packageStatement(path) + .getOrElse(NewFileTemplate.empty), + ) } private def createEmptyFile( - directory: Option[AbsolutePath], + directory: AbsolutePath, name: String, extension: String, ): Future[(AbsolutePath, Range)] = { - val path = directory.getOrElse(workspace).resolve(name + extension) + val path = directory.resolve(name + extension) createFileAndWriteText(path, NewFileTemplate.empty) } diff --git a/tests/unit/src/test/scala/tests/NewFileLspSuite.scala b/tests/unit/src/test/scala/tests/NewFileLspSuite.scala index 66a25c96f29..4d12ddc008f 100644 --- a/tests/unit/src/test/scala/tests/NewFileLspSuite.scala +++ b/tests/unit/src/test/scala/tests/NewFileLspSuite.scala @@ -648,9 +648,12 @@ class NewFileLspSuite extends BaseLspSuite("new-file") { |{ | "a": { "scalaVersion" : "$localScalaVersion" } |} + |/focusedDoc.txt + | |$existingFiles """.stripMargin ) + _ <- server.didFocus("focusedDoc.txt") _ <- server.executeCommand(command, args: _*) _ = { assertNoDiff(