Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibility to render quoted keys? #137

Closed
markus1189 opened this issue Jul 8, 2018 · 11 comments

Comments

@markus1189
Copy link

commented Jul 8, 2018

In dhall-lang/dhall-json#41 there is a problem with quoting keys of objects using the yaml library.

The issue is that string keys are never quoted which might lead to unintentional conversion. For example if you convert a string key "NO" to a yaml like this:

NO: Norway

the NO is now a boolean in YAML and no longer a string. Is there anyway to force the quoting of the key in that case?

@snoyberg

This comment has been minimized.

Copy link
Owner

commented Jul 9, 2018

I'm not opposed to this in theory, we already use such logic for string values of mappings. But I'd like to know where exactly this causes breakage. AIUI, the quoting of strings is never strictly necessary according to the YAML spec, it's only used because some interpretations (including ours!) will eagerly convert from string into a different scalar for values to make things a bit more type-safe and/or match JSON semantics. However, I'm unaware of any library doing the same thing with keys.

@markus1189

This comment has been minimized.

Copy link
Author

commented Jul 9, 2018

My use case is that I have a Dhall record like this:

{ NO: "Norway"
, DE: "Germany"
}

when converting it to yaml using this library, the problem is that NO will be a boolean in the resulting YAML, while DE is still (correctly) a string:

NO: Norway
DE: Germany

To fix this accidental conversion it would be great if one could specify that keys should be quoted.

@snoyberg

This comment has been minimized.

Copy link
Owner

commented Jul 9, 2018

Which library is interpreting the YAML that way? It's definitely not the yaml library itself, since it's mapping to JSON it will always treat keys as string scalars:

#!/usr/bin/env stack
-- stack --resolver lts-11.10 script
{-# LANGUAGE OverloadedStrings #-}
import Control.Exception
import Data.Yaml

main :: IO ()
main = do
  val <- either throwIO pure
       $ decodeEither' "NO: Norway\nDE: Germany\n"
  print (val :: Value)

Results in:

$ stack Main.hs
Object (fromList [("DE",String "Germany"),("NO",String "Norway")])
@markus1189

This comment has been minimized.

Copy link
Author

commented Jul 9, 2018

According to the YAML spec, NO is a boolean, any implementation that adheres to that should interpret

NO: Norway
DE: Boolean

as

false: "Norway"
"DE": "Germany"

Examples that do this: http://www.yamllint.com/, https://yaml-online-parser.appspot.com/ and other implementations like the standard ruby yaml library:

irb(main):001:0> require 'yaml'                                                                                                                                                                                    
=> true                                                                                                                                                                                                            
irb(main):002:0> YAML.load_file('/tmp/norway.yaml')                                                                                                                                                                
=> {false=>"Norway"}

and also pythons pyyaml:

>>> import yaml                                                                                                                                                                                                    
>>> yaml.load(open('/tmp/norway.yaml'))                                                                                                                                                                            
{False: 'Norway'}

(Btw, yes, YAML is weird... 😂 )

@snoyberg

This comment has been minimized.

Copy link
Owner

commented Jul 9, 2018

Alright, I can confirm you're right. Initially, I had been under the impression that the parser had the prerogative to "cast" scalars to either text, bool, or numeric, depending on what succeeded. It seems I was quite mistaken about that, which in turn means that our handling of scalar values is more correct than I realized. On the other hand, quoting the scalar keys is necessary, as you've pointed out. I'll push a commit in a bit.

@snoyberg snoyberg closed this in 5648c22 Jul 9, 2018

snoyberg added a commit that referenced this issue Jul 9, 2018
Merge pull request #138 from snoyberg/137-quoted-keys
Quote keys as necessary (fixes #137)
@hvr

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2018

@markus1189 btw, re "According to the YAML spec, NO is a boolean" -- that's not a true statement; NO definitely not a valid boolean (i.e. tag:yaml.org,2002:bool) representation in the most recent http://yaml.org/spec/1.2/spec.html

I recommend https://hackage.haskell.org/package/HsYAML if you need a standards-compliant YAML parser.

@markus1189

This comment has been minimized.

Copy link
Author

commented Jul 9, 2018

@hvr

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2018

@markus1189 yeah, that spec you're referring to was merely a working draft

@markus1189

This comment has been minimized.

Copy link
Author

commented Jul 9, 2018

@hvr

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2018

@markus1189 It's a long story... basically those aren't YAML 1.2 (released in 2009) compliant implementations but may only be implementing the ancient YAML 1.1 (released in 2005) spec. I'm not sure what yaml implements exactly, but it's certainly isn't YAML 1.2 (HsYAML however implements YAML 1.2).

@snoyberg

This comment has been minimized.

Copy link
Owner

commented Jul 9, 2018

Enough @hvr, no more spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.