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

Add support for optional directives #977

Closed
wants to merge 1 commit into from
Closed

Add support for optional directives #977

wants to merge 1 commit into from

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Feb 8, 2017

No description provided.

@pranavkm
Copy link
Contributor Author

pranavkm commented Feb 8, 2017

Unblocks #968 scenarios for Razor pages

@@ -35,39 +37,58 @@ public DefaultDirectiveDescriptorBuilder(string name, DirectiveDescriptorKind ty
_tokenDescriptors = new List<DirectiveTokenDescriptor>();
}

public IDirectiveDescriptorBuilder AddType()
public IDirectiveDescriptorBuilder AddType(bool optional)
Copy link
Member

@NTaylorMullen NTaylorMullen Feb 8, 2017

Choose a reason for hiding this comment

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

Instead of having each API specify whether its optional or not we could make an API that direct the user into a state where they can't fail easily. Aka:

builder
    .AddType()
    .RemainingTokensOptional()
    .AddString()
    .AddMember();

And

builder
    .AddType()
    .RemainingTokensOptional()
    .AddString()
    .RemainingTokensOptional() // Explode
    .AddMember();

We all know I'm horrible at naming choices but you get the gist 😄.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, sounds work-able.

@@ -1549,7 +1553,13 @@ private void HandleDirective(DirectiveDescriptor descriptor)
outputKind = SpanKind.Code;
break;
case DirectiveTokenKind.String:
AcceptAndMoveNext();
if (!AcceptAndMoveNext())
Copy link
Member

Choose a reason for hiding this comment

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

AcceptAndMoveNext() fails only at EOF right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, might be so.

@@ -734,6 +734,23 @@ public void SectionDirective()
.Accepts(AcceptedCharacters.None)));
}

[Fact(Skip = "I don't know how to write Razor tests"]
Copy link
Member

Choose a reason for hiding this comment

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

wow

@pranavkm
Copy link
Contributor Author

pranavkm commented Feb 8, 2017

🆙 📅

Getting this in to unblock Mvc. We can rev on the design \ code in the PR for #968

@pranavkm pranavkm closed this Feb 8, 2017
@pranavkm pranavkm deleted the prkrishn/968 branch March 13, 2017 18:15
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.

None yet

3 participants