New setting to control creation of Artifact Id folder

[brunovieira97]

New setting spring.initializr.createArtifactIdFolder to customize behavior of project creation.

Since #75, the baseDir URL param is used and set to Artifact ID, causing the project to be generated inside a folder with the Artifact ID as its name. This setting allows the user to disable such behavior, generating the project in the selected folder directly. By default, the folder will be created, to preserve current functionality.

If the Artifact Id folder creation is disabled, I've introduced a simple check to make sure the project destination directory is empty. If it's not, an alert will be shown to prevent overwriting existing files without user consent.

I've also included a minor fix that prevents the following notification to be shown if the generated project folder is already opened in VS Code.

Closes #169

[Eskibear]

@CsCherrYY please take a look.

[CsCherrYY]

here when the user cancels the select folder dialog, outputUri will be undefined, directly accessing outputUri.fsPath would cause an error.

[brunovieira97]

I believe that would happen in the previous code as well, right? Not very experienced in TS.

Anyway, do you have a more specific recommendation for this? Or simply checking if the value is not undefined right after line 112 seems fine?

[Eskibear]

I believe that would happen in the previous code as well,

previous code is safe because it has a outputUri && before, so output.fsPath will not be executed if outputUri is undefined.

Another reason not to use fsPath is, for some virtual workspaces like Codespaces, there no filesystem and uri doesn's starts with scheme file:.

Here you create variable projectLocation only because you want to judge if it's an empty. I suggest to directly use vscode.workspace.fs.readDirectory(uri), which can also cover those virtual workspaces. And you don't need projectLocation then.

[brunovieira97]

I believe my changes now are more aligned with Virtual Workspaces compatibility. Instead of using vscode.Uri.file(), I've created them using .parse(), as the former adds the scheme to path as well, for some weird reason.

When showing the project path in specifyOpenMethod(), it'd be harder to read. Example:

{
    "path": "/file:https:///Users/johndoe/projects/folder",
    "scheme": "file"
}

By using vscode.Uri.parse("/Users/johndoe/projects/folder"), this is the result:

{
    "path": "/Users/johndoe/projects/folder",
    "scheme": "file"
}

[brunovieira97]

I've done a bit of refactoring to simplify stuff. While applying the enum + removing .fsPath usage for my code, I noticed that without auxiliary variables, we'd have some issues dealing with outputUri:

While it was returned in specifyTargetFolder(), the artifactId was not permanently appended to the path until outside of this function, right before unzipping. Any checks inside the mentioned function would be done by manually joining paths without actually modifying the outputUri variable.

So I chose to remove any output/destination folder attributions outside of specifyTargetFolder(), ensuring that's the function responsibility. I removed the param baseDir (that defined artifactId as the folder inside .zip file) from the HTTP call and all the sub-folder logic is done by the extension directly, while extracting the zip, to avoid any "surprises".

Any unnecessary variables (as far as I remember) have been stripped out and the code is better now IMHO. Did some adjustments on URI usage for the rest of the code as well, limited to GenerateProjectHandler.ts.

[Eskibear]

Thanks, overall LGTM, and you tried your best to support virtual workspace. I thought you would simply change baseDir in HTTP call according to the setting. Turns out you choose manually append it to outputUri. A potential concern is, whether extract-zip will help to create the parent folder if it doesn't exist. Please help to check it.

And I find an issue with your bits. With default setting, I tend to create a project demo, target folder is desktop. Error occurs as below.

I guess it might be caused by extract(filepath, { dir: targetFolder.path }, because extract-zip might only support filesystem operations and accept only fsPath.

[brunovieira97]

Yeah, I'm fairly new to extension development, so had to do some research with Uri usage to find out how to better support it, but since I'm running/testing it locally (on macOS), maybe it's not 100% on virtual workspaces.

About extract-zip, on my Mac it created the directory on all of my tests, made sure it would before removing baseDir, but maybe some extra tests are needed from my side. Also, I wasn't able to get any errors on Mac, maybe it's related to how I'm creating vscode.Uri and it's behavior on Windows.

Will take a look later today, as soon as I get my hand on a Windows PC.

---

@Eskibear just some last considerations on baseDir removal... I was going to do like you said and just change it's value based on the setting. For the reasons stated in my previous comment, I went ahead and removed all that complexity when defining the actual outputUri, leveraging extract-zip's automatic folder creation.

My judgment was that just defining outputUri in specifyTargetFolder() was simpler and would avoid confusion if/when the possibilities for spring.initializr.parentFolder are expanded. This way we just increase the logic in one single place and the other points will just use whatever value outputUri has.

If you guys prefer the previous behavior, just let me know.

[Eskibear]

I'm ok with your way of doing this, just ensure this "parent folder" is correctly created, either leveraging extract-zip or do it by our own.

So far the logic LGTM, and the only problem is the path issue mentioned above. I can help to investigate on Windows if I have time.

[brunovieira97]

So, I got my hands on a Windows PC and was able to debug the error you found, @Eskibear. Seems like using vscode.Uri.path is not the way to go. It only works on macOS and Linux (presumably) due to the Unix structure being fairly similar to the one used in this attribute from Uri class. The one we should use (that basically returns the path adapted to the current file system / operating system) is vscode.Uri.fsPath().

I understand compatibility with Virtual Workspaces is not going to be achieved by relying on this method, but we're basically dealing with file system stuff anyway, and .fsPath() is going to be needed in order to maintain multiplatform compatibility.

What I think is the best way to go, is to just leave all URI manipulations in the old way, reversing back to fsPath() usage, and trying to refactor this in a different issue/PR. Seems like it's going to bring up all sorts of challenges, and I fear it'll get in the way of pushing this new feature forward.

Are you guys ok with this approach? If yes, I'll push a new commit with the changes.

---

Side notes regarding zip-extract

One key thing to take into consideration when looking for virtual workspaces compatibility is zip-extract. It only works with a string for the path, and giving it anything different than Uri.fsPath() will cause problems with it when running mkdir $path.

Maybe in the future (assuming you guys agree with me on this matter), we could even bring in our own code to deal with vscode.Uri, or look for an alternative lib.

I'm 100% interested in helping as much as I can on this refactor for virtual workspaces ;)

[CsCherrYY]

An alternative way is to check the scheme of the vscode.Uri here. If the scehme is file, then we can use vscode.Uri.fsPath() to get the correct file path, this can also handle all cases without virtual workspace. And this mechanism also keeps the compatibility with the future virtual workspace, for another uri scheme (maybe virtual workspace scheme such as vscode-vfs), we can have a different approach.

[Eskibear]

@brunovieira97 I agree that we use fsPath for the moment.

to just leave all URI manipulations in the old way, reversing back to fsPath() usage

I like the way you calculate outputUri only in specifyTargetFolder step, e.g. appending artifactId in advance according to settings. IMO we can only revert all outputUri.path to outputUri.fsPath.

BTW I just recall that virtual workspace is partially supported in this extension, and command"create a project" is hidden for virtual workspace. See #186

[Eskibear]

@CsCherrYY A good idea. But I prefer simplicity here in this PR. As mentioned above, we can do that in a different PR to fully support virtual workspace by refactoring all this. But before that, we should find a way to download and extract files in virtual workspace.

[brunovieira97]

I like the way you calculate outputUri only in specifyTargetFolder step, e.g. appending artifactId in advance according to settings. IMO we can only revert all outputUri.path to outputUri.fsPath.

That's the idea. I'll test everything on macOS + Windows here and push as soon as possible.

[brunovieira97]

Sorry for the delay, had a busy week. Just pushed a commit that reverts the URI changes so we can focus only on the actual feature, as discussed.

I've tested the project creation flow on both Windows and macOS, with both setting values, and in all tests the behavior was correct. Feel free to do any further validation and let me know if anything else is needed.

[Eskibear]

LGTM. Thanks!