Add line mappings to project TagHelper attribute values. #244

NTaylorMullen · 2014-11-25T21:53:19Z

We now create LineMappings for instances where a TagHelper's attribute value is not of a string type.
This will enable the Razor editor to create projections from .cshtml => .cs for TagHelper attributes.
Modified the TagHelperCodeGenerator and TagHelperBlockBuiler to accurately track the attribute values start locations so it could flow into code generation.
Modified existing tests to account for the new line mappings.
Tag Helpers: Add design time projections for attributes #207

dougbu · 2014-12-02T02:47:27Z

src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpTagHelperCodeRenderer.cs

        {
            RenderAttributeValue(
                attributeDescriptor,
                valueRenderer: (writer) =>
                {
-                    writer.Write(value);
+                    // We only want to do the work of generating a line mapping if we're in design time mode.


isn't design-time what the CSharpLineMappingWriter(CSharpCodeWriter, SourceLocation, string) constructor overload is for? otherwise it doesn't look like the debugger will have correct line number pragmas

Purposefully not creating the LinePragmas here. Attributes cannot be debugged normally in Razor and after some talks with Damian we've decided to work on supporting that when we address debugability for ordinary Razor attribute code.

The issue in making TagHelpers attributes debuggable ends up being the same problem as normal HTML attributes: The line that starts the generated code is variable length and therefore doesn't result in a good debugging experience (badly placed yellow highlight).

please remove or reword the comment. "want" isn't relevant. at most the comment should say something about generating line pragmas not being worthwhile because debugging C# in attributes has other issues.

dougbu · 2014-12-02T03:03:00Z

⌚

dougbu · 2014-12-05T04:40:33Z

src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperBlockBuilder.cs

@@ -159,6 +159,8 @@ public override void Reset()
                Kind = span.Kind
            };
            var htmlSymbols = span.Symbols.OfType<HtmlSymbol>().ToArray();
+            var capturedAttributeValueStart = false;
+            var attributeValueStartLocation = span.Start;


where exactly will this point when capturedAttributeValueStart == false and when that's true?
I suspect it'll point to the start of the attribute name in the false case and the @ in the true case. I hope that's not correct...

You were correct, addressed this.

NTaylorMullen · 2014-12-11T22:52:43Z

Updated.

pranavkm · 2014-12-16T00:13:57Z

src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperBlockBuilder.cs

                {
-                    name = symbol.Content;
+                    // When symbols are accepted into SpanBuilders their locations get altered to be offset by the 


super nit, split the comment into two lines (or more if you're creative).

When symbols are accepted into SpanBuilders their locations get altered to be offset by the parent. We account for this by marking our start location prior to adding the symbol so we know the location of the attribute value start within the document.

👍 to @pranavkm. I'd go a bit further and add a comma after SpanBuilders and split the second sentence after symbol. Perhaps "Thus" instead of "So" but that's a nano-nit.

dougbu · 2014-12-19T04:42:02Z

src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpTagHelperCodeRenderer.cs

-                    writer.Write(value);
+                    if (_context.Host.DesignTimeMode)
+                    {
+                        using (new CSharpLineMappingWriter(_writer, documentLocation, value.Length))


should this use the lambda parameter writer and not the _writer field? if correct as-is, add a code comment

dougbu · 2014-12-19T05:30:42Z

after at least adding code comment about writer versus _writer or otherwise addressing that question

pranavkm · 2014-12-19T18:46:37Z

src/Microsoft.AspNet.Razor/Parser/TagHelpers/TagHelperBlockBuilder.cs

                        // Set the symbol offset to 0 so we don't attempt to skip an end quote that doesn't exist.
                        symbolOffset = 0;
                    }

+                    attributeValueStartLocation = symbolStartLocation + span.Start + SourceLocation.One;


Is this the only location where SourceLocation.One is used? Doesn't seem like you'd need to define a public field for that purpose.

pranavkm · 2014-12-19T18:53:44Z

once comments are addressed.

- We now create LineMappings for instances where a TagHelper's attribute value is not of a string type. - This will enable the Razor editor to create projections from .cshtml => .cs for TagHelper attributes. - Modified the TagHelperCodeGenerator and TagHelperBlockBuiler to accurately track the attribute values start locations so it could flow into code generation. - Modified existing tests to account for the new line mappings. #207

dougbu reviewed Dec 2, 2014
View reviewed changes

dougbu reviewed Dec 5, 2014
View reviewed changes

NTaylorMullen force-pushed the thprojections.207 branch from 3bafd0d to 7f2ec08 Compare December 11, 2014 22:52

NTaylorMullen force-pushed the thprojections.207 branch from 7f2ec08 to 871e82c Compare December 15, 2014 23:10

NTaylorMullen mentioned this pull request Dec 15, 2014

React to aspnet/Razor#207 changes. aspnet/Mvc#1726

Merged

pranavkm reviewed Dec 16, 2014
View reviewed changes

NTaylorMullen force-pushed the thprojections.207 branch from 871e82c to 6b94a47 Compare December 18, 2014 23:56

dougbu reviewed Dec 19, 2014
View reviewed changes

pranavkm reviewed Dec 19, 2014
View reviewed changes

NTaylorMullen force-pushed the thprojections.207 branch from 6b94a47 to a86b0dc Compare December 22, 2014 19:38

NTaylorMullen merged commit a86b0dc into dev Dec 22, 2014

NTaylorMullen deleted the thprojections.207 branch December 22, 2014 22:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add line mappings to project TagHelper attribute values. #244

Add line mappings to project TagHelper attribute values. #244

NTaylorMullen commented Nov 25, 2014

dougbu Dec 2, 2014

NTaylorMullen Dec 2, 2014

dougbu Dec 5, 2014

dougbu commented Dec 2, 2014

dougbu Dec 5, 2014

NTaylorMullen Dec 11, 2014

NTaylorMullen commented Dec 11, 2014

pranavkm Dec 16, 2014

dougbu Dec 19, 2014

dougbu Dec 19, 2014

NTaylorMullen Dec 20, 2014

dougbu commented Dec 19, 2014

pranavkm Dec 19, 2014

pranavkm commented Dec 19, 2014

Add line mappings to project TagHelper attribute values. #244

Add line mappings to project TagHelper attribute values. #244

Conversation

NTaylorMullen commented Nov 25, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented Dec 2, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NTaylorMullen commented Dec 11, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dougbu commented Dec 19, 2014

Choose a reason for hiding this comment

pranavkm commented Dec 19, 2014