improvement: make new file independent of workspace root

scalameta · Oct 24, 2023 · 9cb2239 · 9cb2239
1 parent b5a2d8d
commit 9cb2239
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 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
@@ -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
@@ -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
@@ -943,15 +943,21 @@ 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
           .map(getServiceFor)
           .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
@@ -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
@@ -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(