timbertson / dhall-render

Render multiple files from dhall expressions
BSD Zero Clause License
18 stars 7 forks source link

Improve handling of top-level arrays in YAML #13

Open SebastianKG opened 3 years ago

SebastianKG commented 3 years ago

The Problem

I have a YAML configuration structure I would like to describe that uses top-level arrays in some files. However, dhall-render breaks in one case, and handles top-level arrays in unexpected ways in other cases.

Sample Code

With a directory structure of:

dhall-render/
    files.dhall

...and files.dhall having contents:

let Render =
      https://raw.githubusercontent.com/timbertson/dhall-render/master/package.dhall

let yaml = \(T : Type) -> \(contents : T) -> (Render.YAMLFile T)::{ contents }

let ListElement = { number : Natural, letters : Text }

in {
     files =
     { dhall-render/render =
         Render.SelfInstall.makeExe
           Render.SelfInstall::{ path = "dhall-render/files.dhall" }
     , `empty.json` = yaml (List ListElement) ([] : List ListElement)
     , `one.yaml` = yaml (List ListElement) [ { number = 1, letters = "one" } ]
     , `many.yaml` =
         yaml
           (List ListElement)
           [ { number = 1, letters = "many.1" }
           , { number = 2, letters = "many.2" }
           ]
     }
   }

Actual Results

When I run the following command and get the following error:

$ echo '(./dhall-render/files.dhall).files.`dhall-render/render`.contents' | dhall text | ruby*** Generating files in `generated` ...
dhall-render/render:
 + ln -sfn ../generated/dhall-render/render dhall-render/render
empty.json:
Error processing document:
{"contents"=>[], "executable"=>false, "format"=>"YAML", "headerFormat"=>"# ", "headerLines"=>["**NOTE**: this file is generated by `dhall-render`.", "You should NOT edit it manually, your changes will be lost."], "install"=>"Symlink"}
Document keys: ["contents", "executable", "format", "headerFormat", "headerLines", "install"]
Traceback (most recent call last):
        19: from -:229:in `<main>'
        18: from -:222:in `main'
        17: from -:222:in `each'
        16: from -:223:in `block in main'
        15: from -:190:in `process'
        14: from /usr/lib/ruby/2.5.0/open3.rb:147:in `popen2'
        13: from /usr/lib/ruby/2.5.0/open3.rb:205:in `popen_run'
        12: from -:193:in `block in process'
        11: from -:175:in `process_json'
        10: from -:119:in `generate_into'
         9: from -:119:in `each_pair'
         8: from -:127:in `block in generate_into'
         7: from -:111:in `block in generate_into'
         6: from -:55:in `block in generate_into'
         5: from -:15:in `block in <main>'
         4: from /usr/lib/ruby/2.5.0/psych.rb:456:in `dump_stream'
         3: from /usr/lib/ruby/2.5.0/psych/visitors/yaml_tree.rb:99:in `tree'
         2: from /usr/lib/ruby/2.5.0/psych/visitors/yaml_tree.rb:93:in `finish'
         1: from /usr/lib/ruby/2.5.0/psych/tree_builder.rb:92:in `end_stream'
/usr/lib/ruby/2.5.0/psych/tree_builder.rb:133:in `set_end_location': undefined method `end_line=' for nil:NilClass (NoMethodError)

So it clearly fails on an empty array. Now, if we remove the line with the "empty.yaml" definition:

...
       { dhall-render/render =
          Render.SelfInstall.makeExe
            Render.SelfInstall::{ path = "dhall-render/files.dhall" }
      , `one.yaml` = yaml (List ListElement) [ { number = 1, letters = "one" } ]
      , `many.yaml` =
          yaml
            (List ListElement)
            [ { number = 1, letters = "many.1" }
            , { number = 2, letters = "many.2" }
            ]
      }
...

...I get the following two files:

one.yaml

# **NOTE**: this file is generated by `dhall-render`.
# You should NOT edit it manually, your changes will be lost.

---
letters: one
number: 1

many.yaml

# **NOTE**: this file is generated by `dhall-render`.
# You should NOT edit it manually, your changes will be lost.

---
letters: many.1
number: 1
---
letters: many.2
number: 2

...but these aren't top-level arrays, they're separate documents.

Expected Result

empty.yaml

[]

one.yaml

- letters: one
  number: 1

many.yaml

- letters: many.1
  number: 1
- letters: many.2
  number: 2

Notes

JSON does not suffer from this issue. All three files are rendered as expected as JSONFiles. Thanks for your help, dhall-render is super cool and a great help when working with Dhall on large projects.

timbertson commented 3 years ago

Thanks for writing up this detailed report!

This was originally added in https://github.com/timbertson/dhall-render/commit/f10494d09ebec352ca7a22888e6401e4d28ae8db, so it's definitely intentional. Well, the failure on an empy array isn't, that sucks 😞

The associated PR is https://github.com/timbertson/dhall-render/pull/5, although I modified the original contribution. It used a new YAMLStream type, instead of assuming a yaml with a toplevel array should be output as a stream. Perhaps it'd be worth reintroducing that distinction, although maybe YAMLDocuments or YAMLMulti would be a better name.

At the time I justified it because if you really want a toplevel array, you could output JSON (all JSON is valid YAML). Would that work for your use case?

SebastianKG commented 3 years ago

The fact that JSON is a subset of YAML is a good point that I hadn't considered, but I do still want that YAML human-readability from my output files so I don't think that would be acceptable for my use case. My preference would be a secondary type as that PR introduced, so that I could just use YAMLFile as it functioned before.

timbertson commented 3 years ago

Sounds fair, if you'd like to submit a PR reintroducing that separate type I'd be happy with that. But let's call it YAMLDocuments. @davidreuss you added this feature in the first place, does that sound good to you?

davidreuss commented 3 years ago

sounds good to me! 👍