Conversation
This removes the `args` from the `ProgressTask` passed to `withProgress`. The `args` is only used by the `commandRunnerWithProgress` and can easily be replaced by an anonymous function that passes the `args` instead. This will simplify the `ProgressTask` interface and make it easier to use.
This will make the progress options passed to `withProgress` optional by moving it to be the second argument and setting a default value for the `location`. This will make it much easier to use from a variety of commands.
| // Make certain properties within a type optional | ||
| type Optional<T, K extends keyof T> = Pick<Partial<T>, K> & Omit<T, K>; | ||
|
|
||
| export type ProgressOptions = Optional<VSCodeProgressOptions, "location">; |
There was a problem hiding this comment.
This is cool, but I'm wondering what the advantage is over having a union of two types: one with the location attribute and one without?
There was a problem hiding this comment.
I can see you want to use VSCodeProgressOptions, but it's harder to read than two types added together.
🤔 Just wondering how to make this easier to understand when you first look at it.
There was a problem hiding this comment.
I didn't want to copy over the types of VSCodeProgressOptions, but you're right that this makes it harder to read. We can copy over the types of that type, but that means potentially some additional work when we update the vscode types, which might not be too bad for this type. What do you think?
There was a problem hiding this comment.
Yeah, ProgressOptions and ProgressLocation don't look that elaborate so it wouldn't be that much work to change. But we would need to remember to do it, which we probably won't unless we have solid testing around what we expect the progress UI to look like.
Since I don't think we have testing at this level, I'm leaning towards just using the root types like you've already done.
3 participants
See the two separate commits. This will make
withProgresseasier to use by removing the required options argument and making it optional instead. This brings it in line with howcommandRunnerWithProgresscalls this function.Checklist
ready-for-doc-reviewlabel there.