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

Add support for parenthesis in "when" conditions #91473

Closed
Veetaha opened this issue Feb 25, 2020 · 18 comments · Fixed by #174471
Closed

Add support for parenthesis in "when" conditions #91473

Veetaha opened this issue Feb 25, 2020 · 18 comments · Fixed by #174471
Assignees
Labels
context-keys feature-request Request for new features or functionality on-testplan
Milestone

Comments

@Veetaha
Copy link

Veetaha commented Feb 25, 2020

  • VSCode Version: 1.42.0
  • OS Version: Ubuntu 18.04.3

The when expression that contains parentheses which we set for a keybinding defined in the contribution point of our extension results into a corrupted (invalidly transformed expression in user's keybinding settings), e.g. the following declaration:

{
    "command": "extension.helloWorld",
    "key": "enter",
    "when": "editorLangId == rust && !suggestWidgetVisible && (!vim.active || vim.mode == 'Insert')"
}

produces the following invalid when expression:

Steps to Reproduce:

  1. Go to the following repo with the reproduction
  2. Follow the steps in README.md

Does this issue occur when all extensions are disabled?: Yes

Veetaha pushed a commit to Veetaha/rust-analyzer that referenced this issue Feb 25, 2020
vscode-vim extension overrides the `type` command so that it prevents
some keypresses to reach the text document editor.
It conflicts with our `onEnter` keybinding
that is used to support automatic doc comments extending and
indentation insertion.

The VSCode-native way to implement this would be
to use regular expressions, but as per matklad it is
considered not acceptable for the language server.

Thus we implement it via a `Enter` keybinding that
invokes our `onEnter` command which sends
a request to rust-analyzer process and applies
the appropriate source change recieved from it.

At the end of the day we may only apply
ad hoc workarounds for conflicting extensions.
But vscode has another bug for that. You
either cannot use parantheses in `when` condition
of a keybinding or it just malfunctions.

See an issue about that here: microsoft/vscode#91473
To get the ultimate context, follow this zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Enhanced.20typing
@sandy081 sandy081 added bug Issue identified by VS Code Team member as probable bug keybindings-editor Keybinding editor issues labels Feb 26, 2020
@sandy081 sandy081 added this to the March 2020 milestone Feb 26, 2020
bors bot added a commit to rust-lang/rust-analyzer that referenced this issue Feb 27, 2020
3308: vscode: fix vscode-vim keybindings conflict r=matklad a=Veetaha

Closes #3013 I hope

vscode-vim extension overrides the `type` command so that it prevents
some keypresses to reach the text document editor.
It conflicts with our `onEnter` keybinding
that is used to support automatic doc comments extending and
indentation insertion.

The VSCode-native way to implement this would be
to use regular expressions, but as per matklad it is
considered not acceptable for the language server.

Thus we implement it via a `Enter` keybinding that
invokes our `onEnter` command which does it via the language-server.

At the end of the day we may only apply
ad hoc workarounds for conflicting extensions.
But vscode has another bug for that. You
either cannot use parantheses in `when` condition
of a keybinding or it just malfunctions.

See an issue about that here: microsoft/vscode#91473
To get the ultimate context, follow this [zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Enhanced.20typing)

Co-authored-by: Veetaha <gerzoh1@gmail.com>
@yasintz
Copy link

yasintz commented Mar 4, 2020

It should be able to properly handle something like
( (a&&(b|| c) && d) ) && !e && false && true || !(f != g) && (h == i).

@sandy081 sandy081 modified the milestones: March 2020, April 2020 Apr 1, 2020
@sandy081 sandy081 assigned jrieken and unassigned sandy081 Apr 2, 2020
@sandy081 sandy081 removed this from the April 2020 milestone Apr 2, 2020
@sandy081 sandy081 removed the keybindings-editor Keybinding editor issues label Apr 2, 2020
@sandy081
Copy link
Member

sandy081 commented Apr 2, 2020

Keybindings editor is rendering the text generated by when context serialize method.

@jrieken jrieken assigned alexdima and unassigned jrieken Apr 2, 2020
@alexdima alexdima removed the bug Issue identified by VS Code Team member as probable bug label Apr 2, 2020
@alexdima
Copy link
Member

alexdima commented Apr 2, 2020

Parenthesis are not supported

@alexdima alexdima changed the title "when" condition with parentheses in keybindings contribution point is corrupted at runtime Add support for paranthesis in "when" conditions Apr 2, 2020
@alexdima alexdima added feature-request Request for new features or functionality context-keys labels Apr 2, 2020
@alexdima alexdima removed their assignment Apr 2, 2020
@alexdima alexdima added this to the Backlog Candidates milestone Apr 2, 2020
@vscodebot
Copy link

vscodebot bot commented Apr 2, 2020

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@alexdima
Copy link
Member

alexdima commented Apr 2, 2020

The workaround is to use boolean algebra and express the same without paranthesis.

@vscodebot
Copy link

vscodebot bot commented Apr 3, 2020

🙂 This feature request received a sufficient number of community upvotes and we moved it to our backlog. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@ulugbekna
Copy link
Contributor

Hi, I have questions regarding this issue:

  1. Would a fix for this issue be a parser that can handle parentheses (arbitrarily nested)?
  2. If yes, would that parser be ideally handwritten or one could use a parser combinator library such as https://github.com/microsoft/ts-parsec?
  3. How do we want to fail if the when clause is malformed, e.g., unmatched parentheses? My understanding is to log the error to console and return false as the evaluation of the clause.

@Spongman
Copy link

Spongman commented Jul 13, 2022

at the very least the Keyboard Shortcuts UI should reject inputs that contain parentheses (if they're not supported currently) instead of garbling the user input resulting in non-functioning expressions and no undo.

@Veetaha Veetaha changed the title Add support for paranthesis in "when" conditions Add support for parenthesis in "when" conditions Jul 14, 2022
@matronator

This comment was marked as spam.

@defpis

This comment was marked as spam.

@starball5
Copy link

Related Stack Overflow question: Is there a NOT operator in VSCode Keybindings JSON to invert an entire "when" clause?

Please also remember to update the docs if/when this is implemented:

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Feb 20, 2023
@ulugbekna ulugbekna modified the milestones: February 2023, March 2023 Feb 20, 2023
@ulugbekna ulugbekna reopened this Feb 20, 2023
@VSCodeTriageBot VSCodeTriageBot removed the unreleased Patch has not yet been released in VS Code Insiders label Feb 20, 2023
@ulugbekna
Copy link
Contributor

ulugbekna commented Mar 8, 2023

Hi everyone! 👋🙂

Support for parentheses in "when" clauses is now available in VS Code Insiders and is planned to be available in the March release of VS Code early next month.

We rewrote the parser to make it more flexible, correct, and easier to change. You can read more about upcoming changes: new features, breakages, migration help in this issue. You can subscribe to that issue to get more updates and/or leave us feedback there.

We also encourage you to test your extensions with VS Code Insiders to make sure things work as you expect, and use the new linter mentioned in the issue above to see if some of the when-clauses you have contain errors.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.