search

maetl / calyx

A Ruby library for generating text with recursive template grammars.
MIT License
62 stars 5 forks source link

File conversions for JSON and YAML #12

Closed tra38 closed 8 years ago

tra38 commented 8 years ago

Submitting this for code review. I may have to refactor some of the test code in the future, especially as we don't know what other edge cases may exist with JSON/YAML. I suspect that we may have more edge cases with JSON than we do with YAML.

I do note that one major difference between the two methods is that load_yml takes the name of a filename while load_json takes the raw JSON file in and of itself. Should both methods behave consistently?

(See Issue #7 for more details)

maetl commented 8 years ago

Yes, I’d definitely say that #load_json and #load_yml should have the same signature and expectations. I would be okay with changing them both to accept a string, and adding a new method #load or #source that accepts a string filename or path object and does a File.read, and calls the appropriate load method based on the file extension of the passed-in path.

tra38 commented 8 years ago

#load has now been created (along with an error message in case someone tries to load a file that Calyx cannot handle yet), so I think this pull request is ready to be merged into master.

maetl commented 8 years ago

:+1: Thanks for this.

I’m gonna have a bit more of a think about whether there are any structural changes that might improve the usability of the formats. Also have a few related API changes I want to consider over time (decoupling some of the features from the main grammar class) but I don’t think any of that blocks the basic addition of this feature, so should be fine to merge now.