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.

Sign up for GitHub

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

Jump to bottom

Expose adjustWhitespace to TextEditor API. #234858

Merged
merged 4 commits into from
Jan 31, 2025
Merged

Expose adjustWhitespace to TextEditor API. #234858

merged 4 commits into from
Jan 31, 2025

Conversation

rgrunber
Copy link
Contributor

@rgrunber rgrunber commented Nov 28, 2024 •
edited
Loading

Based on comments like microsoft/language-server-protocol#1892 (comment) , snippet edits should probably not be adjusting whitespace by default. For language support, once workspace edits have a property like,

vscode/src/vscode-dts/vscode.d.ts

Lines 4996 to 5001 in 275faf6

/**
* Keep whitespace of the {@link CompletionItem.insertText insertText} as is. By default, the editor adjusts leading
* whitespace of new lines so that they match the indentation of the line for which the item is accepted - setting
* this to `true` will prevent that.
*/
keepWhitespace?: boolean;
we could set the snippet insertion to respect that.

Before

insert-snippet-before.mp4

After

insert-snippet-after.mp4

@rgrunber
Copy link
Contributor Author

rgrunber commented Nov 29, 2024 •
edited
Loading

@microsoft-github-policy-service agree company="Red Hat"

jrieken
jrieken requested changes Dec 11, 2024
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this

src/vscode-dts/vscode.d.ts Outdated Show resolved Hide resolved
src/vs/workbench/contrib/bulkEdit/browser/bulkTextEdits.ts Outdated Show resolved Hide resolved
extensions/emmet/src/abbreviationActions.ts Outdated Show resolved Hide resolved
extensions/emmet/src/abbreviationActions.ts Outdated Show resolved Hide resolved
@rgrunber
Expose adjustWhitespace to TextEditor API.
68c2087 
- Disable adjustWhitespace in text edits with snippets so that code
  actions do not adjust snippet indentation by default
- Adjust Emmet extension so that adjustWhitespace is enabled
- Add testcase

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber
Respond to review comments.
4f39e1d 
- Rename adjustWhitespace to keepWhitespace
- Retain default behaviour of adjusting whitespace when keepWhitespace
  not defined

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber
Remove incorrect method parameter default.
ed13a95 
Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber
Introduce 'keepWhitespace' property for snippet text edit abstractions.
8f4b28a 
- Restore original behaviour of bulk text edits adjusting whitespace
- Add testcase

Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@rgrunber rgrunber requested a review from jrieken January 20, 2025 14:46
@jrieken jrieken added this to the February 2025 milestone Jan 31, 2025
@jrieken
Copy link
Member

jrieken commented Jan 31, 2025

Also asking @mjbvz to review because this adds new API without going the proposed api flow

@mjbvz mjbvz merged commit 334fdfe into microsoft:main Jan 31, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@818Nawaf 818Nawaf 818Nawaf approved these changes

@mjbvz mjbvz mjbvz approved these changes

@jrieken jrieken jrieken approved these changes

Assignees

@jrieken jrieken

Labels
None yet
Projects
None yet
Milestone
February 2025
Development

Successfully merging this pull request may close these issues.

editor.insertSnippet() messes with indenting of the SnippetString
4 participants
@rgrunber @jrieken @mjbvz @818Nawaf