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

Fold Shell and Process task? #27587

Closed
jrieken opened this issue May 30, 2017 · 5 comments
Closed

Fold Shell and Process task? #27587

jrieken opened this issue May 30, 2017 · 5 comments
Assignees
Labels
tasks Task system issues under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented May 30, 2017

For the sake of simplify I would like start a discussion about folding ShellTask and ProcessTask. The following things confuse me:

  • A TaskProvider must return a Task but I cannot instantiate a Task and I have to know that a Task is an or-type
  • Shell and process task have 7 identical properties and only differ in process vs commandLine and in options, which again have quite some overlap
  • My understanding is that a ShellTask should also be able to execute a process
  • Does a ShellTask run in the terminal and does a process task "run" in an output channel or both run in a terminal?
@jrieken jrieken added this to the May 2017 milestone May 30, 2017
@dbaeumer
Copy link
Member

dbaeumer commented May 30, 2017

I tried two times to fold them so if you have a good idea I am happy to do so. The underlying problem is args and command line quoting. A process task is not executed in a shell, although it runs in the terminal. It basically reuses the xterm for proper ANSI control character handling (bash at the end of the day is a process as well).

If you create a process task with arguments the process name and the args are passed directly to the OS without any inspection. You don't even need to quote args with blanks. Compared to node this is child_process.execFile. You can do execFile('program', ['arg with blank'])

A ShellTask is executed inside a shell. This brings the hell of argument quoting and since more and more custom shells exist there are different ways to quote arguments. Bash supports three " , ' and \ . Cmd only " but has very special handling if both the command name and arguments have spaces. In the old world I tried to re-implement the argument quoting rules and did so for cmd.exe and bash. But PowerShell is different again and under Linux / Mac there are even a set of new shells. To not be forced to jump after new shells I decided to push the problem to the end user :-). So for shell task user can only give me a command line and need to quote themselves. The node equivalent of a ShellTask is child_process.exec. You need to do exec('program "arg with blank"') or under bash ("program 'arg with blank'") or ('program arg\ with\ blank') depending on what type of quoting you want to achieve.

Users ask for Process support since a shell has influence on the environment (it sources stuff). As said it is like node has exec and execFile.

So the difference is:

Process: command with arg[] & option control the cwd and the env
Shell: commandLine & options control cwd, env and the shell to use

One idea that we could do is to have a task type where the difference is control in the constructor but there is no way to access any of the attributes making the difference via API. So you can't access args, options, process, commandLine via a getter. If you want to change that you need to create a new Task. However since most of the constructor args are strings it might be very hard to distinguish based on the constructor signature.

Do you have any idea? I will sleep over it. May be I have a better idea.

@dbaeumer
Copy link
Member

Possible signatures:

(name: string, process: { executable: string; args: string[]}, options: ProcessOptions, problemMatchers) for process task

(name: string, commandLine: string, options: ShellOptions, problemMatchers) for shell task.

The more I think about this the more I like it.

@jrieken
Copy link
Member Author

jrieken commented May 31, 2017

Unsure about all the details but with Uri we have no (or a private) ctor and just static factory methods like Uri.file and Uri.parse which do the lifting to align different inputs onto the same data structure. I know uri is somewhat simpler than a task but maybe the same trick can be played again

@dbaeumer dbaeumer modified the milestones: June 2017, May 2017 May 31, 2017
@dbaeumer
Copy link
Member

Moving to June. Static helpers might be a good solution here.

@dbaeumer dbaeumer added under-discussion Issue is under discussion for relevance, priority, approach tasks Task system issues labels Jun 6, 2017
@dbaeumer
Copy link
Member

Addressed in the new API.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tasks Task system issues under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants