Remove LocateOwner in formatting #9237
Conversation
| @@ -224,8 +224,7 @@ private static List<TextChange> AdjustRazorIndentation(FormattingContext context | |||
| private static bool IsPartOfHtmlTag(FormattingContext context, int position) | |||
| { | |||
| var syntaxTree = context.CodeDocument.GetSyntaxTree(); | |||
| var change = new SourceChange(position, 0, string.Empty); | |||
| var owner = syntaxTree.Root.LocateOwner(change); | |||
| var owner = syntaxTree.Root.FindInnermostNode(position, includeWhitespace: true); | |||
There was a problem hiding this comment.
Include whitespace may not be needed here...
There was a problem hiding this comment.
If the tests all pass with it as false then no objection here, but I didn't check
| var syntaxTree = context.CodeDocument.GetSyntaxTree(); | ||
| var owner = syntaxTree.Root.LocateOwner(change); | ||
| var owner = syntaxTree.Root.FindInnermostNode(mappingSpan.Start, includeWhitespace: true); |
There was a problem hiding this comment.
@ryzngard I changed this to the overload that just takes a position, to match the old API. You had it as the one that takes a span, and I don't trust that the formatting engine can handle that sort of change, even if it makes sense with the parameters being passed in here.
| IsImplicitExpression() || | ||
| IsInSectionDirectiveCloseBrace() || | ||
| (!allowImplicitStatements && IsImplicitStatementStart())) | ||
| if (IsInsideRazorComment()) |
There was a problem hiding this comment.
Just spread this out to aid debugging as it was impossible to tell which one of these methods was the thing that returned early. One of these "Should have done it years ago" changes.
| @@ -224,8 +224,7 @@ private static List<TextChange> AdjustRazorIndentation(FormattingContext context | |||
| private static bool IsPartOfHtmlTag(FormattingContext context, int position) | |||
| { | |||
| var syntaxTree = context.CodeDocument.GetSyntaxTree(); | |||
| var change = new SourceChange(position, 0, string.Empty); | |||
| var owner = syntaxTree.Root.LocateOwner(change); | |||
| var owner = syntaxTree.Root.FindInnermostNode(position, includeWhitespace: true); | |||
There was a problem hiding this comment.
If the tests all pass with it as false then no objection here, but I didn't check
| @@ -12,7 +12,7 @@ | |||
| <div class="navbar-collapse collapse"> | |||
| <ul class="nav "> | |||
| <li>@Html.ActionLink("Home", "Index", "Home")</li> | |||
| <li>@Html.ActionLink( "About", "About", "Home")</li> | |||
| <li>@Html.ActionLink("About", "About", "Home")</li> | |||
There was a problem hiding this comment.
This was a surprise, but a pretty good change.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
ghost
added this to the 
Also in linked ranges, because no fixes were needed (at least according to tests?)