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

Extra newline no longer rendered at the end of a code block #507

Merged
merged 1 commit into from
Sep 10, 2015

Conversation

ajaybhargavb
Copy link
Contributor

Issue - #485

This fixes the issue with the first example in #485 (comment).
The second example no longer seems to be an issue.
I'm trying various scenarios to see if that issue exists and also to see if this change broke something else. (All tests passing other than the ones updated here)

@NTaylorMullen @dougbu

@@ -261,6 +261,20 @@ public void ParseDocumentIgnoresTagsInContentsOfScriptTag()
}

[Fact]
public void ParseDocumentDoesNotRenderExtraNewLine()

Choose a reason for hiding this comment

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

Add @{ } <html>\r\n variation to ensure that the end newline isn't ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

@NTaylorMullen
Copy link

@ajaybhargavb ajaybhargavb force-pushed the ajbaaska/removewhitespace.485 branch 2 times, most recently from dfd3072 to 743df7a Compare September 8, 2015 22:24
@ajaybhargavb
Copy link
Contributor Author

Updated.

{
PutCurrentBack();
CompleteBlock(insertMarkerIfNecessary: false);

Choose a reason for hiding this comment

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

Shouldn't you always complete the block here whether it's nested or not?

Choose a reason for hiding this comment

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

In the nested situation we're not nulling out the SpanKind.Code's ChunkGenerator. I think this should be fine but we should validate with debugging (make sure not excess whitespace is highlighted) once a WTE is produced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. The name is misleading here. This CompleteBlock does nothing here other than EnsureCurrent and PutCurrentBack. Removing it affects no tests. I am thinking we should remove it completely.

@NTaylorMullen
Copy link

@ajaybhargavb
Copy link
Contributor Author

Updated.

Factory.Markup("\r\n").With(SpanChunkGenerator.Null),
BlockFactory.MarkupTagBlock("<html>")));
}

Choose a reason for hiding this comment

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

Few more tests

  • @{\r\n} \t\r\n<html>
  • @{\r\n@if(true){\r\n} <input> }
  • @{\r\n@if(true){\r\n} \r\n<input> \r\n}<html>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

@NTaylorMullen
Copy link

@ajaybhargavb
Copy link
Contributor Author

Updated.

@NTaylorMullen
Copy link

:shipit:

@ajaybhargavb ajaybhargavb force-pushed the ajbaaska/removewhitespace.485 branch from 7aa7d84 to 5ef10af Compare September 10, 2015 17:37
- #485
- Using a flag to consume whitespace and newline at the end of a verbatim block
- Added tests to validate nested blocks
@ajaybhargavb ajaybhargavb force-pushed the ajbaaska/removewhitespace.485 branch from 5ef10af to e2881b0 Compare September 10, 2015 17:39
@ajaybhargavb ajaybhargavb merged commit e2881b0 into dev Sep 10, 2015
@NTaylorMullen NTaylorMullen deleted the ajbaaska/removewhitespace.485 branch September 24, 2015 17:52
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.

3 participants