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

Allow use of integer variables in tasks.json #95866

Closed
dwilliamson opened this issue Apr 22, 2020 · 7 comments
Closed

Allow use of integer variables in tasks.json #95866

dwilliamson opened this issue Apr 22, 2020 · 7 comments
Assignees
Labels
editor-contrib Editor collection of extras feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@dwilliamson
Copy link
Contributor

I'm trying to call revealLine from a task input as that allows variable substitution (unlike keybindings):

    "inputs": [
        {
            "id": "RevealLine",
            "type": "command",
            "command": "revealLine",
            "args":{
                "lineNumber": "${lineNumber}",
                "at": "center"
            },
        }
    ]

Obviously this fails because the command revealLine expects an integer as the first command. It works fine if I substitute "${lineNumber}" with any old integer.

Of course, this is a valid JSON file with no pre-process and I anticipate you want to keep this concept. But it occurs to me that the type matching between strings an integers could be loosened a bit, allowing this conversion. I may be useful elsewhere.

Thanks again for this great tool.

@alexr00
Copy link
Member

alexr00 commented Apr 23, 2020

This would require that the variable substitution know something about where the variable substitution is happening. Instead, you could very easily write your own command that takes a string lineNumber and converts it to a number before calling the revealLine command.

@alexr00 alexr00 added the *extension-candidate Issue identified as good extension implementation label Apr 23, 2020
@dwilliamson
Copy link
Contributor Author

Sorry, I'm unsure what this means: doesn't the variable substitution already happen in tasks.json? So as far as I'm aware the above piece of code ends up being presented to the command as:

"args":{
                "lineNumber": "123",
                "at": "center"
            },

The problem is that the command itself wants a number type, not a string type:

private _revealLine(lineNumber: number, revealType: VerticalRevealType, scrollType: editorCommon.ScrollType): void {
if (typeof lineNumber !== 'number') {
throw new Error('Invalid arguments');
}
this._sendRevealRange(
new Range(lineNumber, 1, lineNumber, 1),
revealType,
false,
scrollType
);
}

I'm totally aware there are command extensions out there that can service my needs but I don't trust extensions as a rule and prefer to keep those installed to a minimum.

In this case a very simple addition would be to convert from string to integer but that may seem too specific. I've not dug any deeper into the code than this but I hand-wave assumed there'd be some higher level point (e.g.

export const RevealLine: CoreEditorCommand = registerEditorCommand(new class extends CoreEditorCommand {
) that allowed you to be relaxed about the type of inputs you expect.

@alexr00
Copy link
Member

alexr00 commented Apr 23, 2020

@rebornix would you consider changing the reveal line command to handle a string for the lineNumber?

@alexr00 alexr00 reopened this Apr 23, 2020
@alexr00 alexr00 removed the *extension-candidate Issue identified as good extension implementation label Apr 23, 2020
@alexr00 alexr00 assigned rebornix and unassigned alexr00 Apr 23, 2020
@rebornix
Copy link
Member

@alexr00 we can support both number | string.

@rebornix rebornix added the feature-request Request for new features or functionality label Apr 24, 2020
@rebornix rebornix added this to the May 2020 milestone Apr 24, 2020
@rebornix rebornix modified the milestones: May 2020, June 2020 Jun 1, 2020
@rebornix rebornix modified the milestones: June 2020, Backlog Jun 29, 2020
@rebornix rebornix added the editor-contrib Editor collection of extras label Oct 9, 2020
@rebornix rebornix modified the milestones: Backlog, October 2020 Oct 9, 2020
@rebornix
Copy link
Member

Verifier, please test that you are not seeing warnings in keybindings editor when adding below

{
    "key": "cmd+i",
    "command": "revealLine",
    "args": {
      "lineNumber": "70",
      "at": "center"
    }

and also it works as expected

@rebornix rebornix added the verification-needed Verification of issue is requested label Oct 27, 2020
@weinand weinand added the verified Verification succeeded label Oct 28, 2020
@weinand
Copy link
Contributor

weinand commented Oct 28, 2020

The title of this feature request no longer matches what was actually implemented to close the issue.

The feature "Allow use of integer variables in tasks.json" is still not supported.
But the command "revealLine" now supports a string and a number for the line number argument.

I've verified that "revealLine" now accepts a string argument.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-contrib Editor collection of extras feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants
@rebornix @dwilliamson @weinand @alexr00 and others