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

Allow optional quotes around tag helper directives #647

Merged
merged 1 commit into from
Dec 29, 2015

Conversation

pranavkm
Copy link
Contributor

Fixes #636

@pranavkm
Copy link
Contributor Author

cc @NTaylorMullen

@@ -285,7 +289,7 @@ protected void BaseTypeDirective(string noTypeNameError, Func<string, SpanChunkG
Output(SpanKind.Code, AcceptedCharacters.AnyExceptNewline);
}

private void TagHelperDirective(string keyword, ISpanChunkGenerator chunkGenerator)
private void TagHelperDirective(string keyword, Func<string, ISpanChunkGenerator> chunkGeneratorThunk)
Copy link
Member

Choose a reason for hiding this comment

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

chunkGeneratorThunk? lol wha name is that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Such an awesome name. I can change it to chunkGeneratorFactory if that's the more common term in the codebase.

Copy link
Member

Choose a reason for hiding this comment

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

Por favor 😄

@NTaylorMullen
Copy link
Member

You're missing a piece in this PR. The line mappings of the during design time will be off for @addTagHelper, @removeTagHelper and @tagHelperPrefix. Reason: They're calculated using the chunks starting position which is derived from the chunks association.

@NTaylorMullen
Copy link
Member

@pranavkm
Copy link
Contributor Author

Updated

1 similar comment
@pranavkm
Copy link
Contributor Author

Updated

@@ -1,5 +1,5 @@
@addTagHelper something, nice
@removeTagHelper doesntmatter, nice
@addTagHelper "something, nice"
Copy link
Member

Choose a reason for hiding this comment

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

Do this for a few of the design time code generation tests so we can validate line mappings. Should be one for each, add, remove, prefix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok.

@NTaylorMullen
Copy link
Member

⌚ for line mapping tests.

@pranavkm
Copy link
Contributor Author

Updated

@@ -0,0 +1,10 @@
@tagHelperPrefix THS
Copy link
Member

Choose a reason for hiding this comment

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

Whaaa, nononononono. I was saying that you could just intermingle quotes/non-quotes in existing tests that have design-time tests for them already. Same with below. These things don't deserve their own dedicated tests

@pranavkm
Copy link
Contributor Author

Updated

@@ -98,6 +99,8 @@ private void VisitTagHelperDirectiveChunk(string text, Chunk chunk)
Writer.WriteVariableDeclaration("string", TagHelperDirectiveSyntaxHelper, "null");
}

var text = ((Span)chunk.Association).Content.Trim();
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't work does it? What happens if the content has quotes? Wont the values come out double quoted?

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 old code added quotes back. What should this be doing?

@NTaylorMullen
Copy link
Member

@NTaylorMullen
Copy link
Member

:shipit:

@pranavkm pranavkm merged commit e1aa888 into dev Dec 29, 2015
@pranavkm pranavkm deleted the prkrishn/tag-helper-quotes branch December 29, 2015 22:36
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.

4 participants