-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
vscode: add support for valueSelection
in InputBox
#12050
vscode: add support for valueSelection
in InputBox
#12050
Conversation
6b63df0
to
1905e2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind rebasing your changes to the latest master
, just to have the theia.d.ts
styling fixes out of the way.
Signed-off-by: Jonah Iden <[email protected]>
Signed-off-by: Jonah Iden <[email protected]>
Signed-off-by: Jonah Iden <[email protected]>
Signed-off-by: Jonah Iden <[email protected]>
Signed-off-by: Jonah Iden <[email protected]>
d7b9c35
to
8b3eced
Compare
Signed-off-by: Jonah Iden <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonah-iden please take a look at the linting errors due to missing semicolons: https://github.com/eclipse-theia/theia/actions/runs/3884942760/jobs/6628169768#step:6:832.
If you haven't already I'd suggest installing the dbaeumer.vscode-eslint
extension which highlights eslint errors and warnings directly in the editor.
valueSelection
in InputBox
Signed-off-by: Jonah Iden <[email protected]>
Co-authored-by: Paul Maréchal <[email protected]>
Despite what the typings for the getter say, I believe that Please make sure it's all consistent. edit: Apparently TypeScript is happy with different types between setters and getters, so I guess this part is fine? |
your totally right though. ValueSelection can be undefined until the setter is called. I think it was this way in the vs code code |
Signed-off-by: Jonah Iden <[email protected]>
valueSelection
in InputBox
valueSelection
in InputBox
Is done automaticly by the underlying quickInput Signed-off-by: Jonah Iden <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
- The API is marked as supported
- The extension works as expected
if this PR is okay someone else would have to merge it since i don't have write access rights yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputBox#valueSelection
shouldn't be undefined according to the VS Code API.
edit: Turns out vscode.d.ts
allows undefined
but not the web documentation...
Signed-off-by: Jonah Iden [email protected]
What it does
Fixes: #12016.
it adds a new field to the InputBox with which a specific range of text from the InputBox can be slected
How to test
i build a small vscode extension for testing found here: https://github.com/jonah-iden/inputBox-ValueSelection-test
and as vsix value-selection-test-0.0.1.zip
it just opens up a input box which selects all text until the firs space character when the text is changed
Review checklist
Reminder for reviewers