Fix HTML formatting when using tabs #3556
Conversation
davidwengier
requested review from
allisonchou,
NTaylorMullen,
ryanbrandenburg,
sharwell and
a team
as code owners
...r/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/AutoInsert/OnAutoInsertEndpointTest.cs
Show resolved
Hide resolved
| } | ||
|
|
||
| [Fact] | ||
| [WorkItem("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1273468/")] |
There was a problem hiding this comment.
Ah should I have linked this issue to my one-off PR I did to WebTools?
There was a problem hiding this comment.
Are you turning away from your life of crime?
| @@ -336,7 +336,9 @@ protected async Task<List<TextChange>> AdjustIndentationAsync(FormattingContext | |||
| { | |||
| // Given that the HTML formatter ran before this, we can assume | |||
| // HTML is already correctly formatted. So we can use the existing indentation as is. | |||
| razorDesiredIndentation = context.Indentations[i].ExistingIndentation; | |||
There was a problem hiding this comment.
Do we still use ExistingIndentation anywhere / is it still valuable?
There was a problem hiding this comment.
Yeah, still used for sure.
Lets say after the HTML formatter runs, we get back this:
→→∙∙<br />
So thats two tabs, and two spaces, so an indentation of 4, but an indentation size of 10. Well, we run the indentation pass, we now correctly use ExistingIndentationSize to get back an answer of 10 to create our indentation string, but we still need ExistingIndentation to know how many characters to replace when we perform the edit.
There was a problem hiding this comment.
Ahhh awesome explanation, thank you!
Fixes AB#1273468
(and in case that doesn't work, Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1273468/)
When reformatting we used to look at the existing formatting and base changes on that. When formatting things with tabs, that fell down because we'd see
\t\tand think "oh, they want two spaces of indentation". This PR adds a new field so that we can think "oh, they want 8 columns of indentation" instead.