@alexr00 thanks for reviewing. In your opinion do you think glob is in the right place? Or should it be at the top level task Object?

@jasonwilliams I'm on the fence. On the one hand, the glob is a grouping mechanism. On the other hand, group has only ever meant build and test. I lean slightly toward having it on the task object; however, I would recommend you wait to make any changes to where glob is until we have started the API review process since that will help determine where glob should be.

@alexr00 I've added in the changes you've asked for

Thanks for the changes! I noticed that runBuildCommand and runTestCommand have diverged (not your fault), so I refactored them to reduce the code to review. No further changes requested right now. I'll have a new update for you on the API next week.

Hey @alexr00 im guessing this will hopefully role into the march release. Do you know when it gets reviewed?

Hey @alexr00 I've rebased and added updates. I've tested and it's still working as expected.

@alexr00 i do need your help on something. https://github.com/microsoft/vscode/runs/5694971126?check_suite_focus=true fails because I needed to change this check to be triple equals on true. Otherwise all tasks with isDefault will return (even if the value is a glob string). I have a feeling this is not behaviour we want.

However the test seems to use a stringified "true" statement.
I don't know why anyone would ever pass through true as a string and want it to work.

So do we change the test or the code? I could remove that triple equals and the functionality should still work but wondered if that approach is a good idea.

@jasonwilliams that's a bug in the test, it should not be 'true', but true as you said.

I've removed the API changes since we didn't go through full API review and the original feature request doesn't include a request for the API changes.

Everything else looks good. I will test this week and merge.

Sounds good @alexr00 I'm guessing now v1.66 is shipped it's is a good time for this to go in. Let me know if i need to write up any documentation for the release notes or anywhere.

Rebased, and did some final checks, I actually found a bug.
If there are 2 tasks with globs matching and 1 task which has default: true, the default task would run ignoring the glob.
My latest change fixes that.

So order of precedence is (for both test and build):

1. globs have highest priority (assuming there's a match)
2. If 2 glob tasks match the quickPicker will show tasks you can run
3. If 2 glob tasks don't match the quickPicker will show tasks you can run
4. "isDefault": true has second highest priority (if there's no glob match)
5. if none of the glob tasks match then the isDefault task will be ran as a fallback
6. "isDefault: glob|true" if there's no other task matching the same build type. In other words, even if the glob doesn't match this task will run if its the only task set

pings @alexr00

I'm not going to be able to get to this for April. I will look at it first thing after we have branched for release next week though.

@jasonwilliams I reviewed and tested today. I found an issue where the glob doesn't work when there's only one glob that matches. I fixed it with 1bbc930. My test tasks.json:

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "echo txt",
            "type": "shell",
            "command": "echo TextFile",
            "group": {
                "kind": "build",
                "isDefault": "**.txt"
            }
        },
        {
            "label": "echo js",
            "type": "shell",
            "command": "echo JavascriptFile",
            "group": {
                "kind": "build",
                "isDefault": "**.js"
            },
        }
    ]
}

Then I opened a folder with a file.txt and file.js in it. If you have a chance, please take a look at that commit and make sure it's still consistent with what you're expecting. I plan to merge the PR tomorrow.

Thanks @alexr00
Are there any blockers to this being merged? Or has the release not been cut yet?

@jasonwilliams nope, no blockers. Just wanted to give you a chance to review my changes if you wanted.

@jasonwilliams nope, no blockers. Just wanted to give you a chance to review my changes if you wanted.

@alexr00 my apologies, I missed your message above so I didn't test it. But i can take a look at insiders and test there. Thanks for helping me put this feature through.

@alexr00 I was surprised to see no mention of this feature in https://code.visualstudio.com/updates/v1_68#_engineering or the documentation for tasks https://code.visualstudio.com/docs/editor/tasks

Will it be added there? It seems a bit of a hidden feature at the moment

@jasonwilliams thanks for the ping. Not including this in the release notes was an oversight: microsoft/vscode-docs#5401

I will update the docs to include this feature soon.