microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
732 stars 243 forks source link

AL Language Extension's Commands throw errors when run as Command Variables #5859

Open bjarkihall opened 4 years ago

bjarkihall commented 4 years ago

Describe the bug The VS Code Extension for AL contributes commands that don't resolve their promises to strings/undefined/null, see line 189: resolveWithInputAndCommands , where the output is validated and the error is thrown: "Cannot substitute command variable '{0}' because command did not return a result of type string."

I've been trying out Command variables for tasks.json commands but everytime an AL command gets executed get a thrown error (tasks before the AL command and the command itself get executed but nothing after it).

The only related issue I found for this in AL development was in #5262, where this method seems to be the only way to execute tasks before/after AL commands, since preLaunchTask isn't supported in launch configurations.

To Reproduce You can simply create a task in tasks.json referencing e.g. al.incrementalPublishNoDebug:

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "My-RAD",
            "command": "${command:al.incrementalPublishNoDebug}"
        }
    ]
}

This could further be extended to allow pre/post tasks without the need of a custom extension (which would need to execute other tasks before and after something like "await vscode.commands.executeCommand('al.incrementalPublishNoDebug')"):

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "Before-My-RAD",
            "type": "shell",
            "command": "echo before"
        },
        {
            "label": "My-RAD",
            "command": "${command:al.incrementalPublishNoDebug}"
        },
        {
            "label": "After-My-RAD",
            "type": "shell",
            "command": "echo after"
        },
        {
            "label": "My-RAD-With-PrePost",
            "dependsOn": ["Before-My-RAD", "My-RAD", "After-My-RAD"],
            "dependsOrder": "sequence"
        }
    ]
}

Expected behavior No error should have to occur. It seems like AL commands are resolved to booleans (true on success and false when an error is catched) in buildService.js publishContainer but that's already something the Promise indicates when it's resolved vs. rejected, so wouldn't Promise.reject() make more sense then Promise.resolve(false), or did you want to silence erros/warnings? If that's so, you can Promise.resolve() when everything passes but Promise.resolve('Error: Message about what went wrong.'). I'm not sure whether that's the best way of fixing this or making a PR in vscode where booleans (at least true) are also accepted as a resolved command variable, I thought I'd get feedback here first.

Screenshots image

5. Versions: AL Language: 5.0.254558 Business Central: Version: W1 16.0 (Platform 16.0.11233.12061 + Application 16.0.11240.12076)

aniongithub commented 3 years ago

This is VERY relevant. I use a command from, say a CMake extension in tasks.json to build instead of invoking cmake directly in order to use extension settings. This always leaves a nasty error toast even though the command itself executed correctly.

An easy way for VS Code to keep existing handling of return values, but suppress the error message would be using a setting that we could set in settings.json, say:

suppressCommandOutputTypeErrors: true

this would allow workspaces to selectively supress this error if they use commands that don't return strings.