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

Maven goal from context menu gets executed in other projects terminal #68

Closed
adrianmoya opened this issue Jul 5, 2018 · 10 comments
Closed
Assignees
Labels
feature-request New feature or request
Milestone

Comments

@adrianmoya
Copy link

Describe the bug
I have a workspace with multiple maven projects. When I have a terminal open in project A, and I wish to run a maven goal in project B, I use the context menu over the "Maven projects" side window and click the desired goal. A new maven terminal windows open in the root of project A to run maven in project B.

adrian@MY-LAPTOP:~/projects/my-project-A$ "./mvnw" clean -f "/home/adrian/projects/my-project-B/pom.xml"
While the thing work, it's confusing that the terminal ends up open in the other project's directory.

To Reproduce
Steps to reproduce the behavior:

  1. Add two maven projects to a workspace.
  2. Open an integrated terminal in project A.
  3. Right click on project B in the maven projects list and run clean goal.
  4. You end up with the clean goal for project B ran from the root directory of project A.

Expected behavior
I expected the maven terminal to be open in the root of the actual project I'm running the goal for.

Environments (please complete the following information as much as possible):

  • OS: Linux Ubuntu 16.04
  • VS Code version: 1.25.0
  • Extension version: 0.9.0
@Eskibear
Copy link
Member

Eskibear commented Jul 6, 2018

The requirement sounds reasonable. Currently it only creates a terminal using default params. Let me have a check to see whether there is available vscode API to do that.

@Eskibear
Copy link
Member

Eskibear commented Jul 6, 2018

PR is also welcome if you are willing to fix it. Terminal related code is located in VSCodeUI.ts, I guess a URI can be passed to create a terminal with latest API.

@adrianmoya
Copy link
Author

I could try, but have no idea where to begin. I'll take a look during the weekend on how to develop plugins for vscode, so I can take a shot at it.

@Eskibear
Copy link
Member

For the VS Code API, you can specify the param shellPath to do that.

export function createTerminal(name?: string, shellPath?: string, shellArgs?: string[]): Terminal;

Currently this extension use the same terminal named "Maven" for all your projects. Ref: https://github.com/Microsoft/vscode-maven/blob/master/src/Utils.ts#L252

    export function executeMavenCommand(command: string, pomfile: string): void {
        const fullCommand: string = [
            Utils.getMavenExecutable(),
            command.trim(),
            "-f",
            `"${formattedFilepath(pomfile)}"`,
            workspace.getConfiguration("maven.executable", Uri.file(pomfile)).get<string>("options")
        ].filter((x: string) => x).join(" ");
        const name: string = "Maven";    // <------- terminal name
        VSCodeUI.runInTerminal(fullCommand, { name });   // <------- run in terminal, create one if not exists
        updateLRUCommands(command, pomfile);
    }

I think it's better to create terminals for each project (root folder) in the workspace. I suggest we can do the following:

  1. From the pomfile, get the root folder path of the project.
  2. Hash the root folder path as suffix of the terminal name.
  3. Change signature of the util function VSCodeUI.runInTerminal to pass the root folder path as shellPath to create terminal.

What do you think?

@adrianmoya
Copy link
Author

Sounds like a good plan!

@Eskibear Eskibear mentioned this issue Jul 13, 2018
8 tasks
@adrianmoya
Copy link
Author

adrianmoya commented Jul 15, 2018

@Eskibear working on this issue, I came with the idea of cd'ing into the pom.xml folder and then running the command there. But then an idea came to my mind: The reason for cd'ing or using absolute paths is because we open an interactive terminal. And then the user can do whatever in that terminal, and we need to make sure future commands work. But the way eclipse support maven, it doesn't open an interactive terminal but instead an output window with the results of the command.

Have you consider this instead of using the terminal? It would remove the need to open multiple terminals, we can have only an output window for maven commands, like git for example. And each time the user triggers an action, it would cd, execute and show the output. And it wont leave an open terminal for the user.

Thoughts?

@Eskibear
Copy link
Member

VS Code is not designed as an IDE, and transparency is very important. Using output, we spawn a process in the background and redirect the stdout&stderr. Using terminal we simply send a command, and the user **knows clearly exactly what is being executing. **

@Eskibear
Copy link
Member

BTW, shellPath is not the CWD of the shell, but the path of shell itself.

 @param shellPath Optional path to a custom shell executable to be used in the terminal.

So if you really want to implement the feature, I think you have to execute a formatted cd command first. And if you are using "./mvnw" located in the root folder, cd into a subproject folder might cause you not to find the mvnw file.

@Eskibear
Copy link
Member

@adrianmoya Thank you for your suggestion and contribution. I've improved this together with a large PR #83

Now it launches different terminals for each workspaceRootFolder. As for the subprojects, it shares the same terminal with its root project, and CWD remains unchanged when executing maven commands for it, we only use -f parameter to specify the pom path.

Closing this issue.

@Eskibear Eskibear added this to the 1.0.0 milestone Jul 17, 2018
@Eskibear Eskibear added the feature-request New feature or request label Jul 17, 2018
@adrianmoya
Copy link
Author

Thank you for your hard work on this @Eskibear. Sorry I didn't finally open the PR, but I learned a little bit, maybe I can contribute in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants