Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

Rewrite partial parsing logic #2716

Merged
merged 1 commit into from
Nov 9, 2018

Conversation

ajaybhargavb
Copy link
Contributor

#2700

This is the FINAL piece of the end-to-end puzzle. All tests in all Razor assemblies pass.

Up next,

  • Verifying the design time experience in VS
  • Make sure tests in Mvc pass when referencing the feature branch
  • Cleanup

Result = result;
EditedNode = editedNode;
}

public EditResult(PartialParseResultInternal result, SpanBuilder editedSpan)
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 missed removing this and there may be few other unused legacy methods. I'll remove those as part of cleanup.

protected virtual PartialParseResultInternal CanAcceptChange(Span target, SourceChange change)
public virtual bool OwnsChange(Span target, SourceChange change)
{
// Unused. Will be removed soon.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove.


var annotation = node.GetAnnotations().FirstOrDefault(n => n.Kind == key);
return annotation?.Data;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to partial parsing. This was causing an ambiguous reference.


namespace Microsoft.AspNetCore.Razor.Language.Syntax
{
internal static class LegacySyntaxNodeExtensions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The knowledge of how the old Spans relate to the new SyntaxNodes live here.

@@ -91,6 +91,8 @@ public static bool IsTheory
#endif
}

protected int BaselineTestCount { get; set; }
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 needed this to support the cases when we need to test multiple baselines within a single test

{
public class PartialParserTestBase : SyntaxNodeParserTestBase
{
internal override BlockFactory CreateBlockFactory()
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 will go away

factory.Code(Environment.NewLine).AsStatement(),
factory.MetaCode("}").Accepts(AcceptedCharactersInternal.None)),
factory.EmptyHtml()));
VerifyPartialParseTree(manager, changed.GetText(), expectedCode);
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 is an example of a test that has multiple baselines to test.

@@ -0,0 +1,19 @@
RazorDocument - [0..20)::20 - [foo @await Html. baz]
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've verified most of these tests manually. Should be enough to sanity check just a few.

@ajaybhargavb ajaybhargavb force-pushed the ajbaaska/partial-parsing branch from c843716 to ff1c073 Compare November 8, 2018 08:53
Copy link

@NTaylorMullen NTaylorMullen left a comment

Choose a reason for hiding this comment

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

At this point we could probably run all of WTE's tests correct?

{
newTarget = Syntax.InternalSyntax.SyntaxFactory.UnclassifiedTextLiteral(builder.ToList()).CreateRed(target.Parent, target.Position);
}

Choose a reason for hiding this comment

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

Should an else with a Debug.Fail for the unexpected scenario

throw new ArgumentNullException(nameof(node));
}

var spanNodes = node.DescendantNodes().Where(n => n.IsSpanKind());

Choose a reason for hiding this comment

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

Just ot make this semi-more performant change the logic to yield return because you bail out early a lot of the time in your previous/next methods.

@ajaybhargavb
Copy link
Contributor Author

At this point we could probably run all of WTE's tests correct?

Yup. I am working on it.

@ajaybhargavb ajaybhargavb merged commit ff1c073 into feature/razor-parser Nov 9, 2018
@ajaybhargavb ajaybhargavb deleted the ajbaaska/partial-parsing branch November 9, 2018 00:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants