Add @addtaghelper directive. #142

NTaylorMullen · 2014-09-17T06:05:43Z

Also added some infrastructure pieces for it such as the ITagHelperDescriptorResolver and the default implementation TagHelperDescriptorResolver which will be filled out in a later commit.
Reworked some extensibility points to allow accessibility of the descriptor resolvers per offline discussions.
Fixed existing tests to work with new RazorParser.
Validated directive syntax tree creation, errors and code generation.
Tag Helpers: Add @addtaghelper directive and link to registration system #111

NTaylorMullen · 2014-09-17T06:06:14Z

I can break this into two PR's if people feel that it helps with the code review process.

NTaylorMullen · 2014-09-17T06:17:42Z

src/Microsoft.AspNet.Razor/Generator/Compiler/CodeTree/CodeTreeBuilder.cs

@@ -38,6 +38,14 @@ public void AddChunk(Chunk chunk, SyntaxTreeNode association, bool topLevel = fa
            }
        }

+        public void AddAddTagHelperChunk(string lookupText, SyntaxTreeNode association)


Kinda rough naming but still felt like it made sense. Feel free to object.

naming here derives from @addtaghelper naming so it makes sense. could of course change the keyword to something like @usetaghelper (and later @removetaghelper might be @ignoretaghelper).
/cc @DamianEdwards

dougbu · 2014-09-27T21:43:58Z

...spNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpDesignTimeHelpersVisitor.cs

        internal const string InheritsHelper = "__inheritsHelper";
        internal const string DesignTimeHelperMethodName = "__RazorDesignTimeHelpers__";
+        private static readonly string ObjectTypeString = typeof(object).ToString();


why not just generate the string "object"?

or string or (if you do initialization) var

dougbu · 2014-09-27T22:43:11Z

on PR description "Validated directive syntax tree creation, errors and code generation." bullet, looks as if you are only newly validating the @addtaghelper directive. correct?

dougbu · 2014-09-27T22:49:42Z

test/Microsoft.AspNet.Razor.Test/Generator/CSharpTagHelperRenderingTest.cs

+                    tabTest: TabTest.NoTabs,
+                    expectedDesignTimePragmas: new List<LineMapping>()
+                    {
+                        BuildLineMapping(14, 0, 440, 14, 14, 11)


icky magic numbers. please at least name the parameters

NTaylorMullen · 2014-10-03T08:06:04Z

Addressed code review comments.

dougbu · 2014-10-03T15:31:18Z

...spNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpDesignTimeHelpersVisitor.cs

+
+            Writer.WriteStartAssignment(TagHelperDirectiveSyntaxHelper);
+
+            _csharpCodeVisitor.CreateExpressionCodeMapping(


(let's help the children 😺) add comments referring reader to code in CSharpCodeParser.TagHelperDirective() that ensures LookupText is not surrounded by the quotes. Might also be worth mentioning this restores quotes that code requires then removes.

dougbu · 2014-10-03T16:42:42Z

⌚ very close; will mainly look at comments and nameof() use next time.

NTaylorMullen · 2014-10-04T00:28:47Z

Addressed code review comments.

dougbu · 2014-10-04T04:11:02Z

...spNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpDesignTimeHelpersVisitor.cs

        private const int DisableVariableNamingWarnings = 219;

-        public CSharpDesignTimeHelpersVisitor(CSharpCodeWriter writer, CodeBuilderContext context)
-            : base(writer, context) { }
+        private CSharpCodeVisitor _csharpCodeVisitor;


add readonly

dougbu · 2014-10-04T05:05:56Z

after adding a couple of readonly modifiers and addressing the few non-nit open comment comments

- Also added some infrastructure pieces for it such as the ITagHelperDescriptorResolver and the default implementation TagHelperDescriptorResolver which will be filled out in a later commit. - Reworked some extensibility points to allow accessibility of the descriptor resolvers per offline discussions. #111

- Fixed existing tests to work with new RazorParser. - Validated directive syntax tree creation, errors and code generation. #111

NTaylorMullen changed the title ~~Add @addtaghelper directive.~~ [Design] Add @addtaghelper directive. Sep 17, 2014

NTaylorMullen reviewed Sep 17, 2014
View reviewed changes

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from b918f8b to e8673f8 Compare September 17, 2014 18:21

NTaylorMullen force-pushed the TagHelpers_AttributeCodeGeneration branch from 3f2fa4a to 6a99ea3 Compare September 19, 2014 06:53

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from e8673f8 to 4cd59eb Compare September 19, 2014 06:53

NTaylorMullen changed the title ~~[Design] Add @addtaghelper directive.~~ Add @addtaghelper directive. Sep 20, 2014

NTaylorMullen force-pushed the TagHelpers_AttributeCodeGeneration branch from 6a99ea3 to c80b01f Compare September 23, 2014 06:46

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from 4cd59eb to 68fd42a Compare September 23, 2014 06:47

NTaylorMullen force-pushed the TagHelpers_AttributeCodeGeneration branch from c80b01f to af6249b Compare September 24, 2014 00:11

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from 68fd42a to 8ba1682 Compare September 24, 2014 00:11

NTaylorMullen force-pushed the TagHelpers_AttributeCodeGeneration branch from af6249b to f0d0d67 Compare September 25, 2014 07:06

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from 8ba1682 to 2f775d4 Compare September 25, 2014 07:07

NTaylorMullen force-pushed the TagHelpers_AttributeCodeGeneration branch from f0d0d67 to 7eb0f98 Compare September 25, 2014 23:20

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from 2f775d4 to 369a300 Compare September 25, 2014 23:20

NTaylorMullen force-pushed the TagHelpers_AttributeCodeGeneration branch from 7eb0f98 to 4f5e86d Compare September 26, 2014 20:36

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from 369a300 to e9df146 Compare September 26, 2014 20:37

dougbu reviewed Sep 27, 2014
View reviewed changes

NTaylorMullen force-pushed the TagHelpers_AttributeCodeGeneration branch from 040a3a1 to 7399531 Compare October 3, 2014 08:04

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from 145b3c4 to f3d2585 Compare October 3, 2014 08:04

dougbu reviewed Oct 3, 2014
View reviewed changes

dougbu reviewed Oct 4, 2014
View reviewed changes

NTaylorMullen added 2 commits October 4, 2014 11:28

Add tests to validate @addtaghelper directive.

da3051a

- Fixed existing tests to work with new RazorParser. - Validated directive syntax tree creation, errors and code generation. #111

NTaylorMullen force-pushed the TagHelpers_AddDirective branch from 84f0011 to da3051a Compare October 4, 2014 19:52

NTaylorMullen closed this Oct 4, 2014

NTaylorMullen deleted the TagHelpers_AddDirective branch October 4, 2014 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add @addtaghelper directive. #142

Add @addtaghelper directive. #142

NTaylorMullen commented Sep 17, 2014

NTaylorMullen commented Sep 17, 2014

NTaylorMullen Sep 17, 2014

dougbu Sep 27, 2014

dougbu Sep 27, 2014

dougbu Sep 27, 2014

dougbu commented Sep 27, 2014

dougbu Sep 27, 2014

NTaylorMullen commented Oct 3, 2014

dougbu Oct 3, 2014

dougbu commented Oct 3, 2014

NTaylorMullen commented Oct 4, 2014

dougbu Oct 4, 2014

dougbu commented Oct 4, 2014


		Writer.WriteStartAssignment(TagHelperDirectiveSyntaxHelper);

		_csharpCodeVisitor.CreateExpressionCodeMapping(

Add @addtaghelper directive. #142

Add @addtaghelper directive. #142

Conversation

NTaylorMullen commented Sep 17, 2014

NTaylorMullen commented Sep 17, 2014

NTaylorMullen Sep 17, 2014

Choose a reason for hiding this comment

dougbu Sep 27, 2014

Choose a reason for hiding this comment

dougbu Sep 27, 2014

Choose a reason for hiding this comment

dougbu Sep 27, 2014

Choose a reason for hiding this comment

dougbu commented Sep 27, 2014

dougbu Sep 27, 2014

Choose a reason for hiding this comment

NTaylorMullen commented Oct 3, 2014

dougbu Oct 3, 2014

Choose a reason for hiding this comment

dougbu commented Oct 3, 2014

NTaylorMullen commented Oct 4, 2014

dougbu Oct 4, 2014

Choose a reason for hiding this comment

dougbu commented Oct 4, 2014