Skip to content

Proposal: #if(string) to not render if string is blank#36

Merged
tanner0101 merged 1 commit intovapor:masterfrom
bygri:if-blank-string
Dec 9, 2016
Merged

Proposal: #if(string) to not render if string is blank#36
tanner0101 merged 1 commit intovapor:masterfrom
bygri:if-blank-string

Conversation

@bygri
Copy link
Copy Markdown

@bygri bygri commented Dec 9, 2016

This is something of a matter of opinion.

Currently, if a Node represents a blank string, using that node as the argument to #if will cause the block to render. I believe that #if should consider a blank string to be equivalent to false, in this case, and therefore not render.

In Polymorphic a bool has three states: true, false or nil, and "" becomes nil, which makes sense. In this tag, "" is considered not-nil and so returns true, rendering the block.

This patch and test causes #if("") to not render its block.

The alternative, and it's also a valid one, is to force the user of the #if tag to convert empty strings to false or nil before rendering.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 9, 2016

Current coverage is 99.19% (diff: 100%)

Merging #36 into master will increase coverage by 0.01%

@@             master        #36   diff @@
==========================================
  Files            50         50          
  Lines          1352       1369    +17   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1341       1358    +17   
  Misses           11         11          
  Partials          0          0          

Powered by Codecov. Last update 7d0433b...b7c98bb

@tanner0101 tanner0101 added the enhancement New feature or request label Dec 9, 2016
@tanner0101
Copy link
Copy Markdown
Member

This looks great, thanks!

@tanner0101 tanner0101 merged commit 483e364 into vapor:master Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants