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

Commit

Permalink
Addressed code review comments.
Browse files Browse the repository at this point in the history 
#75
  • Loading branch information
@NTaylorMullen
NTaylorMullen committed Aug 21, 2014
1 parent 3303648 commit 23dada9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 27 deletions.
30 changes: 17 additions & 13 deletions src/Microsoft.AspNet.Razor/Parser/HtmlMarkupParser.Block.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,12 @@ private void TagBlock(Stack<Tuple<HtmlSymbol, SourceLocation>> tags)
// Output everything prior to the OpenAngle into a markup span
Output(SpanKind.Markup);

IDisposable tagBlock = null;
// Do not want to start a new tag block if we're at the end of the file.
IDisposable tagBlockWrapper = null;
if (!EndOfFile && !AtSpecialTag)
{
// Start a Block tag. This is used to wrap things like <p> or <a class="btn"> etc.
tagBlock = Context.StartBlock(BlockType.Tag);
tagBlockWrapper = Context.StartBlock(BlockType.Tag);
}

if (EndOfFile)
Expand All @@ -155,7 +155,7 @@ private void TagBlock(Stack<Tuple<HtmlSymbol, SourceLocation>> tags)
}
else
{
complete = AfterTagStart(tagStart, tags, tagBlock);
complete = AfterTagStart(tagStart, tags, tagBlockWrapper);
}
}

Expand All @@ -169,10 +169,10 @@ private void TagBlock(Stack<Tuple<HtmlSymbol, SourceLocation>> tags)
Output(SpanKind.Markup);

// Will be null if we were at end of file or special tag when initially created.
if (tagBlock != null)
if (tagBlockWrapper != null)
{
// End tag block
tagBlock.Dispose();
tagBlockWrapper.Dispose();
}
}
while (tags.Count > 0);
Expand Down Expand Up @@ -342,6 +342,7 @@ private bool EndTextTag(HtmlSymbol solidus, IDisposable tagBlockWrapper)
return seenCloseAngle;
}

// Special tags include <! and <? tags
private bool AtSpecialTag
{
get
Expand Down Expand Up @@ -548,7 +549,8 @@ private void AttributeValue(HtmlSymbolType quote)
sym.Type != HtmlSymbolType.WhiteSpace &&
sym.Type != HtmlSymbolType.NewLine &&
sym.Type != HtmlSymbolType.Transition &&
// This condition checks for the end of the attribute value (it repeats some of the checks above but for now that's ok)
// This condition checks for the end of the attribute value (it repeats some of the checks above
// but for now that's ok)
!IsEndOfAttributeValue(quote, sym));
Accept(value);
Span.CodeGenerator = new LiteralAttributeCodeGenerator(prefix.GetContent(prefixStart), value.GetContent(prefixStart));
Expand Down Expand Up @@ -761,8 +763,6 @@ private bool RestOfTag(Tuple<HtmlSymbol, SourceLocation> tag,
Accept(ws);
Output(SpanKind.Markup); // Output the whitespace

bool complete;

using (Context.StartBlock(BlockType.Tag))
{
Accept(openAngle);
Expand All @@ -772,7 +772,7 @@ private bool RestOfTag(Tuple<HtmlSymbol, SourceLocation> tag,
// Accept to '>', '<' or EOF
AcceptUntil(HtmlSymbolType.CloseAngle, HtmlSymbolType.OpenAngle);
// Accept the '>' if we saw it. And if we do see it, we're complete
complete = Optional(HtmlSymbolType.CloseAngle);
var complete = Optional(HtmlSymbolType.CloseAngle);

if (complete)
{
Expand All @@ -781,17 +781,17 @@ private bool RestOfTag(Tuple<HtmlSymbol, SourceLocation> tag,

// Output the closing void element
Output(SpanKind.Markup);
}

return complete;
return complete;
}
}
}

// Go back to the bookmark and just finish this tag at the close angle
Context.Source.Position = bookmark;
NextToken();
}
else if (String.Equals(tagName, "script", StringComparison.OrdinalIgnoreCase))
else if (string.Equals(tagName, "script", StringComparison.OrdinalIgnoreCase))
{
CompleteTagBlockWithSpan(tagBlockWrapper, AcceptedCharacters.None, SpanKind.Markup);

Expand Down Expand Up @@ -829,8 +829,9 @@ private void SkipToEndScriptAndParseCode(AcceptedCharacters endTagAcceptedCharac
seenEndScript = true;
}

// We put everything back because we just wanted to look ahead to see if the current end tag that we're parsing is
// the script tag. If so we'll generate correct code to encompass it.
PutCurrentBack(); // Put back whatever was after the solidus

PutBack(solidus); // Put back '/'
PutBack(openAngle); // Put back '<'
}
Expand Down Expand Up @@ -864,6 +865,9 @@ private void CompleteTagBlockWithSpan(IDisposable tagBlockWrapper,
AcceptedCharacters acceptedCharacters,
SpanKind spanKind)
{
Debug.Assert(tagBlockWrapper != null,
"Tag block wrapper should not be null when attempting to complete a block");

Span.EditHandler.AcceptedCharacters = acceptedCharacters;
// Write out the current span into the block before closing it.
Output(spanKind);
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNet.Razor/Parser/HtmlMarkupParser.Document.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ private void ScanTagInDocumentContext()

AcceptAndMoveNext(); // Accept '<'

// Tag Start
if (!At(HtmlSymbolType.Solidus))
{
// Parsing a start tag
var scriptTag = At(HtmlSymbolType.Text) &&
string.Equals(CurrentSymbol.Content, "script", StringComparison.OrdinalIgnoreCase);
Optional(HtmlSymbolType.Text);
Expand All @@ -80,7 +80,7 @@ private void ScanTagInDocumentContext()
}
else
{
// Tag End
// Parsing an end tag
// This section can accept things like: '</p >' or '</p>' etc.
Optional(HtmlSymbolType.Solidus);
// Whitespace here is invalid (according to the spec)
Expand Down
18 changes: 6 additions & 12 deletions src/Microsoft.AspNet.Razor/Utils/DisposableAction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading;

namespace Microsoft.AspNet.Razor.Utils
{
internal class DisposableAction : IDisposable
{
private Action _action;
private bool _invoked;

public DisposableAction(Action action)
{
Expand All @@ -17,22 +17,16 @@ public DisposableAction(Action action)
throw new ArgumentNullException("action");
}
_action = action;
_invoked = false;

This comment has been minimized.

Sign in to view

Copy link
@dougbu

dougbu Aug 22, 2014

Member

remove default initialization
}

protected virtual void Dispose(bool disposing)
public void Dispose()
{
// If we were disposed by the finalizer it's because the user didn't use a "using" block, so don't do anything!
if (disposing)
if (!_invoked)
{
// Only ever invoke the action once.
Interlocked.Exchange(ref _action, () => { }).Invoke();
_action();
_invoked = true;
}
}

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
}
}

0 comments on commit 23dada9

Please sign in to comment.