Skip to content

Boolean 'false' fields are not displayed#52

Merged
emhoracek merged 2 commits intomasterfrom
dont-display-false-fields
Jul 20, 2017
Merged

Boolean 'false' fields are not displayed#52
emhoracek merged 2 commits intomasterfrom
dont-display-false-fields

Conversation

@emhoracek
Copy link
Member

Also, update to use latest Larceny.

@emhoracek emhoracek requested a review from dhartunian July 20, 2017 19:37
boolFieldTrue = object [ "person" .= object [ "name" .= ("Ada Lovelace" :: Text )]]

boolFieldFalse :: Value
boolFieldFalse = object [ "person" .= False ]
Copy link
Contributor

Choose a reason for hiding this comment

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

i think in our case the value is the string "false" not the boolean false

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so... I did check but let me verify.

Right i -> rawTextFill $ tshow (i :: Integer)
jsonToFill (Bool b) = rawTextFill $ tshow b
jsonToFill (Bool True) = rawTextFill $ tshow True
jsonToFill (Bool False) = rawTextFill "<!-- JSON field found, but value is false. -->"
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think we want all boolean false values to render this, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't imagine ever wanting "false" (or even "true")

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have another idea?

@emhoracek emhoracek merged commit cf52e3d into master Jul 20, 2017
@emhoracek emhoracek deleted the dont-display-false-fields branch July 20, 2017 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants