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

[VS Code] Fix various formatting issues #8318

Merged
merged 4 commits into from
Feb 24, 2023
Merged

Conversation

allisonchou
Copy link
Contributor

Summary of the changes

@allisonchou allisonchou requested a review from a team as a code owner February 24, 2023 00:33
const htmlFoldingRanges = await vscode.commands.executeCommand<vscode.FoldingRange[]>('vscode.executeFoldingRangeProvider', virtualHtmlUri);
const csharpFoldingRanges = await vscode.commands.executeCommand<vscode.FoldingRange[]>('vscode.executeFoldingRangeProvider', virtualCSharpUri);

const convertedHtmlFoldingRanges = htmlFoldingRanges === undefined ? new Array<FoldingRange>() : this.convertFoldingRanges(htmlFoldingRanges);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added improved error handling in our folding range provider just in case.

const serializableTextEdits = Array<SerializableTextEdit>();
for (const textEdit of textEdits) {
for (let textEdit of textEdits) {
// The below workaround is needed due to a bug on the HTML side where
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should resolve dotnet/vscode-csharp#5561

// Our OnTypeFormatter doesn't do anything at the moment, but it's needed so
// VS Code doesn't throw an exception when it tries to send us an
// OnTypeFormatting request.
vscodeType.languages.registerOnTypeFormattingEditProvider(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should resolve #8175

Copy link
Contributor

Choose a reason for hiding this comment

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

Why dont we support on type formatting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just don't think we ever did in VS Code? We should support it eventually and I don't think (crossing fingers) it should be too difficult. However, it would definitely require a decent amount of validation so I left it out of this PR for now. I filed a tracking issue here: #8329

@@ -46,8 +46,24 @@ export class FormattingHandler {
virtualHtmlUri,
formattingParams.options);

if (textEdits === undefined) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check should (hopefully) resolve #7038

// The below workaround is needed due to a bug on the HTML side where
// they'll sometimes send us an end position that exceeds the length
// of the document. Tracked by https://github.com/microsoft/vscode/issues/175298.
if (textEdit.range.end.line > lineCount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing range.end.line is 1-based, but thought I'd ask to confirm that this shouldn't be >=

Copy link
Contributor Author

Choose a reason for hiding this comment

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

range.end.line is actually 0-based, but I made lineCount also be 0-based. Maybe I should rename it because I see how it could be unclear 😅

@rhuanbarros
Copy link

Hi, how can I set this update in my windows? There is a release?

@khasmir04
Copy link

I have the same question, I just started with blazor template and this shows up everytime, how do we use this update?

@danielscatigno-ncpc
Copy link

danielscatigno-ncpc commented Feb 28, 2023

@rhuanbarros @khasmir04 I Think there wasn't a release yet!
You can follow releases here Releases
But at the moment the last one is 1.25.4 and there is no beta

@KlomDark
Copy link

KlomDark commented Mar 2, 2023

So the VSCode 1.76 release came out yesterday, but this fix does not appear to be in it. Any idea how it got missed?

@rhuanbarros
Copy link

I think that the update will be in the C# Extension.
https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.csharp

@KlomDark
Copy link

KlomDark commented Mar 2, 2023

I think that the update will be in the C# Extension. https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.csharp

Thanks! Still wrapping my head around what does what with this. I will keep watching for an update to the C# extension.

@rhuanbarros
Copy link

@rhuanbarros @khasmir04 I Think there wasn't a release yet! You can follow releases here Releases But at the moment the last one is 1.25.4 and there is no beta

Do you know when we will get a realease? This bug is killing me

@KlomDark
Copy link

@rhuanbarros @khasmir04 I Think there wasn't a release yet! You can follow releases here Releases But at the moment the last one is 1.25.4 and there is no beta

Do you know when we will get a realease? This bug is killing me

Looking at the lack of check-ins lately, I'm really worried that the team is discouraged by Microsoft's announcement that they are making their own C# plug-in. I hope that fear is misplaced and they are just taking some time to reorient their strategy on how to utilize the new Language Server Protocol. I kow Miguel De Icaza is pretty upset about the whole mess. (https://visualstudiomagazine.com/articles/2022/06/16/csharp-vs-code-tool.aspx)

@rhuanbarros
Copy link

Ok, thak you. I didn't know that. Sorry.

@allisonchou
Copy link
Contributor Author

Hi all! We have a release in progress that is currently going through validation. I'll circle back here when the new version is released. Thanks for your patience!

@tkoenig89
Copy link

Hi, I'm not realy sure if i should adress this here. If not please let me know if I should create a new issue. But as this adresses a prerelease i was not quite sure if this was the right thing either.

As a result of this formatting issue i was back at version 1.25.2 for a while and just tried the prerelease version of 1.25.5 which should include these fixes if I'm not wrong. But sadly i still get formatting error messages using the current prerelease.

To ensure it is not related to my project i created a new one (dotnet v7.0.103, C# ext v1.25.5):

dotnet new webapp -n razor-format-test

Trying to format the Index.cshtml document gives the error message 'Request textDocument/formatting failed'. Razor Log shows:

Entering method razor/mapToDocumentRanges. # a few lines of this
Entering method textDocument/formatting.
[Error - 9:55:49 AM] [null]
[Error - 9:55:49 AM] Request textDocument/formatting failed.
  Message: Specified argument was out of the range of valid values. (Parameter 'Range end line 11 matches or exceeds SourceText boundary 11.')
  Code: -32000 
[object Object]

... where line 11 is the last line of the file.

I can provide more info if required, but didnt want to spam here if it is the wrong place.

@lonix1
Copy link

lonix1 commented Apr 12, 2023

Are these fixes meant to be released soon, or only in November - the prerelease page say ".NET 8" 😮

@tkoenig89
Copy link

tkoenig89 commented Apr 12, 2023

@lonix1 I was refering to the release of the vscode extension here which should not be bound to .Net 8 i guess. And this release 1.25.5 lists the formatting issues as solved, which i found not to be solved.

@lonix1
Copy link

lonix1 commented Apr 12, 2023

Can someone shed light on when it will be released? Is it still "going through validation" as mentioned above?

@rhuanbarros
Copy link

rhuanbarros commented Apr 13, 2023

I tested the last realese on windows, but it still throw errors on VScode.
But I could get to minimize the errors by using Code Behind for all Razor files and setting the inherit both in the Razor file and in the razor.cs. (It should be the same to .cshtml file).

TestComponent.razor file

...
@inherits BaseCustomComponent
...

TestComponent.razor.cs file

...
public partial class TestComponent: ComponentBase
...

This way the parser gets the base class.
Hope this help someone.

By the way, You could manual install the realease this way:
image

@DustinCampbell
Copy link
Member

Hello friends! Could you create a new issue instead of commenting on an old, closed PR? Thanks!

@KlomDark
Copy link

KlomDark commented Apr 13, 2023 via email

@DustinCampbell
Copy link
Member

How bout you answer the question please?

I see multiple questions above, all of which are on a PR that was closed nearly two months ago. Please files issues for the questions that are still relevant. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants