-
Notifications
You must be signed in to change notification settings - Fork 29k
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
CustomExecution2 Feedback: expose variable resolvers #81007
Comments
@bwateratmsft Edit: This changes the task ID, which causes a whole host of problems. |
Can you outline a situation in which you couldn't use VS Code API to get the current workspace folder or current file from within your task? |
File and folder are just some of the available variables. There are commands, configs, prompts, environment variables, every imagineable thing about workspace/file/line...end users would expect all of these to work regardless of how the task is implemented behind the scenes. If the extension implementer had to implement support for all of these, it would be very difficult (and probably not even possible in some cases), and it would essentially just end up being a reimplementation of what VSCode has already. |
I think I'm still missing something. Why would you need to use the |
Why would you not need Let me give an example. In the Docker extension, we cannot realistically make ASP.NET Core Blazor web apps work without CustomExecution, because of the order of execution of a debug launch. We have to do some steps after the But, having CustomExecution does not suddenly mean that users don't need to configure the Users have already stated the need to be able to pass in So, unless we get an API that allows us to resolve task variables, here are our choices:
|
Just to add: the ability to resolve tokens is not exclusive to the use of CustomExecution. If the user makes use of tokens such as We're doing this already in our providers, for the tokens we know are being used. But, as the features gain adoption, we can be sure more tokens will be used, and we'd rather not reimplement the wheel. |
@bwateratmsft it sounds like you want to pass values from tasks.json into tasks, which is related #58836. If we implement #58836 then we can easily resolve variables in the passed in values. Would that be enough? |
I think that that implementation might be insufficient. We are building extremely complicated As an example, this relatively simple
becomes this ShellExecution command:
As you can see, the input from the task cannot be trivially mapped into the command line. If we wanted a "simple" task definition that was just that huge command line, without all the complicated logic, then we may as well not implement a task at all. The goal is to make it easier for users to configure the I don't really understand why this sounds so difficult to implement? The heart of the code is already all there, we just need to be able to access it as extension developers. |
Looks like we have an issue for exposing the configuration resolver API already: #2809 |
Thanks for creating this issue! We figured it's covering the same as another one we already have. Thus, we closed this one as a duplicate. You can search for existing issues here. See also our issue reporting guidelines. Happy Coding! |
@bwateratmsft What if we resolved all the variables in your TaskDefinition and then passed that back into the custom execution callback? export class CustomExecution {
/**
* @param process The [Pseudoterminal](#Pseudoterminal) to be used by the task to display output.
* @param callback The callback that will be called when the task is started by a user.
*/
constructor(callback: (resolvedDefinition?: TaskDefinition) => Thenable<Pseudoterminal>);
} |
That sounds great, I can't think of any scenarios where that wouldn't work. It gives extension developers the freedom to use the resolved config, or not use it if they're certain it isn't needed, or mix the two. One question--is this something that has to ship with CustomExecution from the start, or can it be added later? My thinking is that if it meant delaying CustomExecution, I'd rather release what we have now and add this later, if that is possible. |
It's an optional parameter, so I would add it later. The new parameter would sit in vscode.proposed.d.ts for a few months then we would try to finalize it. CustomExecution would not be delayed because of this. |
Moving to November since we are taking October for issue grooming. |
This should work for the Docker extension. Would it be possible to make it object-recursive? Otherwise we can pretty easily deal with the recursion ourselves. |
After much discussion, we are back to the tasks specific API proposal: vscode/src/vs/vscode.proposed.d.ts Lines 989 to 999 in 2d9ffda
Summary of discussion:
|
Folks who are interested in this API, I merged the change that will allow input and command variables to resolve with the |
I will be finalizing this API this week, please speak now if this API doesn't fulfill your custom execution variable resolving needs! https:/microsoft/vscode/blob/master/src/vs/vscode.proposed.d.ts#L1000-L1010 |
Hi, @alexr00 Before it is implemented, how should I get the name of the currently opened |
If your task is a |
|
@kaysonwu what version of VS Code are you using? If you aren't using the latest Insiders build, please do try that since there are important changes there. |
I used |
@kaysonwu please try with the latest insiders: https://code.visualstudio.com/insiders/ |
okay. thanks. |
@alexr00 @bwateratmsft is on leave at the moment, so I don't know how soon we'll be able to check out the new API (cc @karolz-ms) That said, looking at it, it seems like it should work for the bulk of our needs in |
I trust @philliphoff judgement on this |
I'm back! I will take a look today. Update: works great, thanks @alexr00! My PR for it: microsoft/vscode-docker#2250 |
One shortcoming I realized with CustomExecution2 is that each task would have to implement their own way of resolving common variables--e.g. ${workspaceFolder} or ${file}, etc.
Can we get an API to reach the variable resolvers e.g. ConfigurationResolverService? This way any variable input to CustomExecution2 tasks would not require the task to reimplement the wheel.
The text was updated successfully, but these errors were encountered: