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

TextEdit resolution does not work in single server completion #6618

Closed
NTaylorMullen opened this issue Jul 21, 2022 · 0 comments
Closed

TextEdit resolution does not work in single server completion #6618

NTaylorMullen opened this issue Jul 21, 2022 · 0 comments
Assignees
Labels
bug Something isn't working user story
Milestone

Comments

@NTaylorMullen
Copy link
Contributor

This represents trying to complete C# override statements and the await keyword.

@NTaylorMullen NTaylorMullen added the bug Something isn't working label Jul 21, 2022
@NTaylorMullen NTaylorMullen added this to the 17.4-Preview1 CC:~7/25 milestone Jul 21, 2022
@NTaylorMullen NTaylorMullen self-assigned this Jul 21, 2022
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- Updated C# portions of our delegated completion list provider tests to utilze the real C# server and simplified the testing model.
- Maintained a lot of pre-existing tests because we don't have similar capabilities for HTML
- These tests will be more widely used upon completion of #6618 where we'll want to resolve text edits (this is a leadup).

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- Updated C# portions of our delegated completion list provider tests to utilze the real C# server and simplified the testing model.
- Maintained a lot of pre-existing tests because we don't have similar capabilities for HTML
- These tests will be more widely used upon completion of #6618 where we'll want to resolve text edits (this is a leadup).

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- Prior to this C# snippets were wrapped/unwrapped at the endpoint layer. This means anyone trying to format a snippet would need to manually wrap/unwrap their own snippets. The logic fit well into the existing formatting service so I migrated the logic there.
- Another aspect of this that's a little disconnected is that when applying formatted snippet edits I found that edits wouldn't get "minimialized" prior to collapsing which would result in unminified edits returning to the client. At completion time (what I will eventually be doing as part of #6618) this was highly troublesome because completion is sensitive to the "end of the edit" area. Meaning if you complete `await` and the await has a complex text edit that gets formatted the `t` in `await` needs to be the absolute last thing in the edit to not obstruct the cursor; however, without the minimal edit change that doesn't actually work properly. I imagine this was just an unintentional oversight and wasn't quite sure how to test this (although I do test it in the completion resolve tests that are coming latere) so I included it here.
- Currently there's no direct tests of this method it looks like. Although in a follow up PR for #6618 I'll be formatting resolved completion items.

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- Updated C# portions of our delegated completion list provider tests to utilze the real C# server and simplified the testing model.
- Maintained a lot of pre-existing tests because we don't have similar capabilities for HTML
- These tests will be more widely used upon completion of #6618 where we'll want to resolve text edits (this is a leadup).

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- Prior to this C# snippets were wrapped/unwrapped at the endpoint layer. This means anyone trying to format a snippet would need to manually wrap/unwrap their own snippets. The logic fit well into the existing formatting service so I migrated the logic there.
- Another aspect of this that's a little disconnected is that when applying formatted snippet edits I found that edits wouldn't get "minimialized" prior to collapsing which would result in unminified edits returning to the client. At completion time (what I will eventually be doing as part of #6618) this was highly troublesome because completion is sensitive to the "end of the edit" area. Meaning if you complete `await` and the await has a complex text edit that gets formatted the `t` in `await` needs to be the absolute last thing in the edit to not obstruct the cursor; however, without the minimal edit change that doesn't actually work properly. I imagine this was just an unintentional oversight and wasn't quite sure how to test this (although I do test it in the completion resolve tests that are coming latere) so I included it here.
- Currently there's no direct tests of this method it looks like. Although in a follow up PR for #6618 I'll be formatting resolved completion items.

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
…time.

- Migrated our old `CompletionResolutionHandler` logic for post-processing C# completion items to our new single server completion system.
    - One current gap is that the old system used to lookup active formatting options on the client to understand if snippets should be formatted with/without tabs etc. For now I'm using defaults but in a follow up PR i'll light up the real formatting options acquisition logic.
- As part of this PR there were several pieces of code that could be re-used so I refactored them out. The `TestRazorFormattingService` is a prime example (especially now that completion resolve depends on it).
    - Did a few updates to the API so it was clear what type of formatting service you'd be getting (aka should it be HTML enabled?).
- Added a test that validates that we get and remap text edit completions properly (I use `await`). I couldn't find a corresponding C# completion item that utilizes `AdditionalTextEdit`s.

## Enables

![gif of await and override completion working](https://i.imgur.com/EAwqM3j.gif)

Most of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- This is the final part to #6618 to enable completion resolve to acquire formatting options for complex text edits so it can properly format text edits
- We now expose a new endpoint on the client under `razor/formatting/options` that will return the most recent understanding of formatting options
- To make this possible updated the `FormattingOptionsProvider` API to take a `Uri` instead of a document snapshot.
- Updated tests accordingly

Final part of #6618
ghost pushed a commit that referenced this issue Jul 22, 2022
…6624)

- Updated C# portions of our delegated completion list provider tests to utilze the real C# server and simplified the testing model.
- Maintained a lot of pre-existing tests because we don't have similar capabilities for HTML
- These tests will be more widely used upon completion of #6618 where we'll want to resolve text edits (this is a leadup).

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- The original `CSharp_Operator_Triggered` test was an incorrect scenario. For instance, C# doesn't typically trigger completions off of typing `(`. Instead I migrated the test to do a `DateTime.` variant with `.` being the trigger. The only catch then was that our generated C# document did not contain `using System` so I had to update our generated code document bits to properly generate the using statement.

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- Prior to this C# snippets were wrapped/unwrapped at the endpoint layer. This means anyone trying to format a snippet would need to manually wrap/unwrap their own snippets. The logic fit well into the existing formatting service so I migrated the logic there.
- Another aspect of this that's a little disconnected is that when applying formatted snippet edits I found that edits wouldn't get "minimialized" prior to collapsing which would result in unminified edits returning to the client. At completion time (what I will eventually be doing as part of #6618) this was highly troublesome because completion is sensitive to the "end of the edit" area. Meaning if you complete `await` and the await has a complex text edit that gets formatted the `t` in `await` needs to be the absolute last thing in the edit to not obstruct the cursor; however, without the minimal edit change that doesn't actually work properly. I imagine this was just an unintentional oversight and wasn't quite sure how to test this (although I do test it in the completion resolve tests that are coming latere) so I included it here.
- Currently there's no direct tests of this method it looks like. Although in a follow up PR for #6618 I'll be formatting resolved completion items.

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
…time.

- Migrated our old `CompletionResolutionHandler` logic for post-processing C# completion items to our new single server completion system.
    - One current gap is that the old system used to lookup active formatting options on the client to understand if snippets should be formatted with/without tabs etc. For now I'm using defaults but in a follow up PR i'll light up the real formatting options acquisition logic.
- As part of this PR there were several pieces of code that could be re-used so I refactored them out. The `TestRazorFormattingService` is a prime example (especially now that completion resolve depends on it).
    - Did a few updates to the API so it was clear what type of formatting service you'd be getting (aka should it be HTML enabled?).
- Added a test that validates that we get and remap text edit completions properly (I use `await`). I couldn't find a corresponding C# completion item that utilizes `AdditionalTextEdit`s.

## Enables

![gif of await and override completion working](https://i.imgur.com/EAwqM3j.gif)

Most of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- This is the final part to #6618 to enable completion resolve to acquire formatting options for complex text edits so it can properly format text edits
- We now expose a new endpoint on the client under `razor/formatting/options` that will return the most recent understanding of formatting options
- To make this possible updated the `FormattingOptionsProvider` API to take a `Uri` instead of a document snapshot.
- Updated tests accordingly

Final part of #6618
NTaylorMullen added a commit that referenced this issue Jul 22, 2022
- The original `CSharp_Operator_Triggered` test was an incorrect scenario. For instance, C# doesn't typically trigger completions off of typing `(`. Instead I migrated the test to do a `DateTime.` variant with `.` being the trigger. The only catch then was that our generated C# document did not contain `using System` so I had to update our generated code document bits to properly generate the using statement.

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 25, 2022
…time. (#6626)

* Add support for remapping TextEdits & AdditionalTextEdits at resolve time.

- Migrated our old `CompletionResolutionHandler` logic for post-processing C# completion items to our new single server completion system.
    - One current gap is that the old system used to lookup active formatting options on the client to understand if snippets should be formatted with/without tabs etc. For now I'm using defaults but in a follow up PR i'll light up the real formatting options acquisition logic.
- As part of this PR there were several pieces of code that could be re-used so I refactored them out. The `TestRazorFormattingService` is a prime example (especially now that completion resolve depends on it).
    - Did a few updates to the API so it was clear what type of formatting service you'd be getting (aka should it be HTML enabled?).
- Added a test that validates that we get and remap text edit completions properly (I use `await`). I couldn't find a corresponding C# completion item that utilizes `AdditionalTextEdit`s.

## Enables

![gif of await and override completion working](https://i.imgur.com/EAwqM3j.gif)

Most of #6618

* Address code review comments.
NTaylorMullen added a commit that referenced this issue Jul 25, 2022
- This is the final part to #6618 to enable completion resolve to acquire formatting options for complex text edits so it can properly format text edits
- We now expose a new endpoint on the client under `razor/formatting/options` that will return the most recent understanding of formatting options
- To make this possible updated the `FormattingOptionsProvider` API to take a `Uri` instead of a document snapshot.
- Updated tests accordingly

Final part of #6618
NTaylorMullen added a commit that referenced this issue Jul 25, 2022
- This is the final part to #6618 to enable completion resolve to acquire formatting options for complex text edits so it can properly format text edits
- We now expose a new endpoint on the client under `razor/formatting/options` that will return the most recent understanding of formatting options
- To make this possible updated the `FormattingOptionsProvider` API to take a `Uri` instead of a document snapshot.
- Updated tests accordingly

Final part of #6618
NTaylorMullen added a commit that referenced this issue Jul 25, 2022
- Prior to this C# snippets were wrapped/unwrapped at the endpoint layer. This means anyone trying to format a snippet would need to manually wrap/unwrap their own snippets. The logic fit well into the existing formatting service so I migrated the logic there.
- Another aspect of this that's a little disconnected is that when applying formatted snippet edits I found that edits wouldn't get "minimialized" prior to collapsing which would result in unminified edits returning to the client. At completion time (what I will eventually be doing as part of #6618) this was highly troublesome because completion is sensitive to the "end of the edit" area. Meaning if you complete `await` and the await has a complex text edit that gets formatted the `t` in `await` needs to be the absolute last thing in the edit to not obstruct the cursor; however, without the minimal edit change that doesn't actually work properly. I imagine this was just an unintentional oversight and wasn't quite sure how to test this (although I do test it in the completion resolve tests that are coming latere) so I included it here.
- Currently there's no direct tests of this method it looks like. Although in a follow up PR for #6618 I'll be formatting resolved completion items.

Part of #6618
NTaylorMullen added a commit that referenced this issue Jul 25, 2022
…time. (#6626)

* Add support for remapping TextEdits & AdditionalTextEdits at resolve time.

- Migrated our old `CompletionResolutionHandler` logic for post-processing C# completion items to our new single server completion system.
    - One current gap is that the old system used to lookup active formatting options on the client to understand if snippets should be formatted with/without tabs etc. For now I'm using defaults but in a follow up PR i'll light up the real formatting options acquisition logic.
- As part of this PR there were several pieces of code that could be re-used so I refactored them out. The `TestRazorFormattingService` is a prime example (especially now that completion resolve depends on it).
    - Did a few updates to the API so it was clear what type of formatting service you'd be getting (aka should it be HTML enabled?).
- Added a test that validates that we get and remap text edit completions properly (I use `await`). I couldn't find a corresponding C# completion item that utilizes `AdditionalTextEdit`s.

## Enables

![gif of await and override completion working](https://i.imgur.com/EAwqM3j.gif)

Most of #6618

* Address code review comments.
NTaylorMullen added a commit that referenced this issue Jul 25, 2022
- This is the final part to #6618 to enable completion resolve to acquire formatting options for complex text edits so it can properly format text edits
- We now expose a new endpoint on the client under `razor/formatting/options` that will return the most recent understanding of formatting options
- To make this possible updated the `FormattingOptionsProvider` API to take a `Uri` instead of a document snapshot.
- Updated tests accordingly

Final part of #6618
@ghost ghost locked as resolved and limited conversation to collaborators Aug 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working user story
Projects
None yet
Development

No branches or pull requests

2 participants