Streamline Tasks

[jrieken]

Both tasks ShellTask and TerminalTask use a happy mix of readonly, xyz | undefined type declarations, and non JS-ish defaults. I'd recommend to clarify things, e.g

• Use question mark for types that can be undefined, e.g CompletionItem#insertText
• Don't use readonly the model is anyway that VS Code created a disconnected copy of that data providers return
• Don't use empty defaults like an empty array or empty object. The JavaScript programmer expects undefined

[dbaeumer]

Regarding undefined and question mark. In TS there is a difference between this and I thought the difference makes sense to express different behavior:

By using | undefined I wanted to express that a task has an identifier but that undefined is a legal value in which case I will compute one. Same for source. I thought using ? means there can be no identifier / source which is not correct. Using the TS difference I thought is a good thing to do. I also noticed that other parts of the API do the same, for example: StatusBarItem

Any objection to keep this?

Regarding readonly: the idea is that a user needs to create a new task if he wants to change these. But for consistency it makes sense to remove this.
Edit: StatusBarItem uses readonly as well :-)

Regarding empty defaults. I thought it is easier to use if you want to set a property or add an argument later. You can write tasks.args.push(....). If the return value can be undefined you need to write something like this:

let args = task.args | [];
args.push(value);
task.args = args;

Same for option literals.

If you feel strong about I will change it, but I thought it makes the API harder to use.

[jrieken]

Comparing this with StatusBarItem isn't happy because you cannot get a valid status bar item unless you call createStatusBarItem which is then a live object in the main side with a constant location. That's why it uses readonly-ness and explicit undefined-types to communicate what and how it can be configured.

You should compare a Task to something like a CompletionItem or TreeItem which are just data holder objects serving no other purpose than being filled-in by an extension (and read by the API implementation). Constructors of these object should help to create a valid instance but there is reason why it should make some props readonly. Also TypeScript will accept any object shape that looks like one of these things.

In TS there is a difference between this and I thought the difference makes sense to express different behavior:

I learnt something about TypeScript today 👨‍🏫 Tho unsure about the value, the lack of a property and explicitly assigning it with undefined are effectively the same. Staying in the sample a.y === undefined and b.y === undefined so unsure how much it helps that we make people explicitly spell out undefined. Unsure about the trade-off between complexity and more type-safely-win. We should quiz some people about it

[dbaeumer]

Moving to June.

[dbaeumer]

Addressed in the new API.