Configuration and loading from yaml files #61

Aceeri · May 16, 2016

Allows using macros to create a structure along with loading a yaml file into that struct.

It isn't entirely finished as I'd like to add a couple more loading types and clean it up a bit, but it would be very helpful if anyone can spot something I missed or give suggestions.

Basic enum loading? Complex enums would take a rather large feat of macro-ness (tokenizer hackiness?) to get working and I don't see much benefit so it might be better to only allow really basic enums e.g.
enum Example { Option1, Option2, Option3 }
Hashmap loading
Vec loading
Writing to files?
Option loading? null return None, else Some(T)?

I'm going to be a bit busy until next wednesday, so I probably won't be making any changes until then.

Aceeri · May 16, 2016

Aceeri reviewed May 16, 2016

View changes

Aceeri · May 16, 2016

src/config.rs

+
+        #[derive(Clone, Debug)]
+        pub struct $root {
+            _meta: ConfigMeta,


Should I move this to be completely invisible/in the background to the user or keep it exposed? It may be useful if we want to do something like writing to the configs as then we would still have the location of file along with the location the Config struct is defined.

Aceeri · May 16, 2016

Aceeri reviewed May 16, 2016

View changes

Aceeri · May 16, 2016

src/config.rs

+                    path = path + element;
+                }
+
+                format!("{}: Failed to parse YAML: {}: expect {}", meta.path.display(), path, meta.ty)


Should probably re-add in a check if the key is a "BadValue", if it then say "Could not find YAML" instead of "Failed to parse YAML".

lschmierer · May 18, 2016

lschmierer reviewed May 18, 2016

View changes

lschmierer · May 18, 2016

src/config.rs

+
+#[derive(Clone, Debug)]
+pub struct ConfigMeta {
+    path: PathBuf, // Where the file is located, "" if not from a file.


Maybe use Option<PathBuf>. here ;)
A little bit cleaner.

Decided to make it default to "config\config.yml" instead as a default directory for amethyst.

Writing to files? Not sure how necessary this would be.

I think existing Yaml files should be automatically updated to match the layout of the config!{...} defined Configs. So, when the config struct is changed, the developer does not need to take care of the Yaml config files.

lschmierer · May 18, 2016

Writing to files? Not sure how necessary this would be.

I think existing Yaml files should be automatically updated to match the layout of the config!{...} defined Configs. So, when the config struct is changed, the developer does not need to take care of the Yaml config files.

Looks like this is almost done, couple of rather minor bugs to fix. Currently writing will only write to a separate file if the file already exists in config/, otherwise it will just overwrite the "extern" with the default value.

Aceeri · May 20, 2016

Looks like this is almost done, couple of rather minor bugs to fix. Currently writing will only write to a separate file if the file already exists in config/, otherwise it will just overwrite the "extern" with the default value.

Does writing keep comments in the yaml files?

lschmierer · May 21, 2016

Does writing keep comments in the yaml files?

@lschmierer Not at the moment, hadn't thought of that.

Aceeri · May 21, 2016

@lschmierer Not at the moment, hadn't thought of that.

I think that from_file_raw must result in an error if there is there is both display.yml and display/config.yml

thiolliere · May 22, 2016

I think that from_file_raw must result in an error if there is there is both display.yml and display/config.yml

it can also be cool in case display.yml have a key not in its structure definition to warn unexpected key.

thiolliere · May 22, 2016

it can also be cool in case display.yml have a key not in its structure definition to warn unexpected key.

Alright! I added a warning for unexpected keys in the yaml. from_file() now outputs a warning/error if there are multiple files and defaults instead of loading. And finally, documentation for configuration structures and enums work properly now in the macro, along with custom derives so there is less need for comments inside of the .yml/.yaml files.

So far I've tested on windows and linux and it works great.

Aceeri · Jun 1, 2016

Alright! I added a warning for unexpected keys in the yaml. from_file() now outputs a warning/error if there are multiple files and defaults instead of loading. And finally, documentation for configuration structures and enums work properly now in the macro, along with custom derives so there is less need for comments inside of the .yml/.yaml files.

So far I've tested on windows and linux and it works great.

kvark · Jun 7, 2016

kvark reviewed Jun 7, 2016

View changes

kvark · Jun 7, 2016

Cargo.toml

+path = "src/config/"
+version = "0.1.0"
+
+[dependencies]


missing new line

kvark · Jun 10, 2016

kvark reviewed Jun 10, 2016

View changes

kvark · Jun 10, 2016

src/config/examples/main.rs

+use std::path::Path;
+
+fn main() {
+    let config = amethyst_config::Config::from_file(Path::new("../../config/config.yml"));


If you use AsRef<Path>, like File::open does, you could avoid Path::new() here.

kvark · Jun 10, 2016

kvark reviewed Jun 10, 2016

View changes

kvark · Jun 10, 2016

src/config/src/definitions.rs

+            if index != 0 {
+                tree = tree + "->";
+            }
+


You do seem to like empty lines a lot :)

Just a habit I have, I usually separate things that modify scope. I should probably clean up some of these whitespaces though.

kvark · Jun 10, 2016

kvark reviewed Jun 10, 2016

View changes

kvark · Jun 10, 2016

src/config/src/yaml.rs

+                }
+            }
+
+            if array.len() > 10 {


declare 10 as some sort of a constant for clarity?

kvark · Jun 10, 2016

kvark reviewed Jun 10, 2016

View changes

kvark · Jun 10, 2016

src/config/src/yaml.rs

+yaml_array!(7 => 0 1 2 3 4 5 6);
+yaml_array!(8 => 0 1 2 3 4 5 6 7);
+yaml_array!(9 => 0 1 2 3 4 5 6 7 8);
+yaml_array!(10 => 0 1 2 3 4 5 6 7 8 9);


ah, this is where 10 came from, I see

Thanks @Aceeri !

kvark · Jun 15, 2016

Thanks @Aceeri !

Aceeri added some commits May 17, 2016

Optional FromYaml

61cfb95

Failed vs Couldn't find error messages, enum parsing, options message…
… for enums

525d8f9

Vec loading

42c7f7a

Adds FromYaml for maps and sets

41897ee

Aceeri added some commits May 21, 2016

f32/f64 missing fraction fix, documentation explaining macro and how …
…it works

9ca9421

Ignore documentation code

406294c

Aceeri referenced this pull request May 21, 2016
Closed
RFC: Config.ron Specification #29

Aceeri added some commits May 31, 2016

Implemented Element for Tuples

cb938a5

Added tuples up to 10

3e49c9d

Aceeri added some commits Jun 7, 2016

Adds trailing whitespace, minor changes to yaml formatting

6e55a3b

Remove testing config

4a39982

Trailing newline on cargo and config

45917af

Lib newline

37fd7f0

Generic AsRef<Path> for from_file functions

a89cdb2

kvark merged commit 34a746a into amethyst:develop Jun 15, 2016
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

amethyst/amethyst

Join GitHub today

Configuration and loading from yaml files #61

Conversation

Aceeri commented May 16, 2016 • edited Edited 1 time Aceeri edited May 20, 2016 (most recent)

Aceeri added some commits May 14, 2016

Aceeri reviewed May 16, 2016 View changes

This comment has been minimized.

Aceeri May 16, 2016 • edited Edited 1 time Aceeri edited May 16, 2016 (most recent)

Aceeri reviewed May 16, 2016 View changes

This comment has been minimized.

Aceeri added some commits May 17, 2016

lschmierer reviewed May 18, 2016 View changes

This comment has been minimized.

lschmierer May 18, 2016 • edited Edited 1 time lschmierer edited May 18, 2016 (most recent)

This comment has been minimized.

This comment has been minimized.

lschmierer commented May 18, 2016 • edited Edited 1 time lschmierer edited May 18, 2016 (most recent)

Aceeri added some commits May 18, 2016

This comment has been minimized.

Aceeri commented May 20, 2016 • edited Edited 1 time Aceeri edited May 20, 2016 (most recent)

Aceeri added some commits May 21, 2016

Aceeri referenced this pull request May 21, 2016

This comment has been minimized.

lschmierer commented May 21, 2016

This comment has been minimized.

Aceeri commented May 21, 2016

This comment has been minimized.

thiolliere commented May 22, 2016

This comment has been minimized.

thiolliere commented May 22, 2016

Aceeri added some commits May 23, 2016

Aceeri added some commits May 31, 2016

This comment has been minimized.

Aceeri commented Jun 1, 2016 • edited Edited 1 time Aceeri edited Jun 1, 2016 (most recent)

Aceeri added some commits Jun 1, 2016

kvark reviewed Jun 7, 2016 View changes

This comment has been minimized.

Aceeri added some commits Jun 7, 2016

kvark reviewed Jun 10, 2016 View changes

This comment has been minimized.

kvark reviewed Jun 10, 2016 View changes

This comment has been minimized.

This comment has been minimized.

Aceeri Jun 11, 2016 • edited Edited 1 time Aceeri edited Jun 11, 2016 (most recent)

kvark reviewed Jun 10, 2016 View changes

This comment has been minimized.

kvark reviewed Jun 10, 2016 View changes

This comment has been minimized.

This comment has been minimized.

kvark commented Jun 15, 2016

Hide details View details kvark merged commit 34a746a into amethyst:develop Jun 15, 2016 1 check passed

1 check passed

Aceeri commented May 16, 2016 •

edited

Edited 1 time

Aceeri edited May 20, 2016 (most recent)

Aceeri reviewed May 16, 2016

View changes

Aceeri May 16, 2016 •

edited

Edited 1 time

Aceeri edited May 16, 2016 (most recent)

Aceeri reviewed May 16, 2016

View changes

lschmierer reviewed May 18, 2016

View changes

lschmierer May 18, 2016 •

edited

Edited 1 time

lschmierer edited May 18, 2016 (most recent)

lschmierer commented May 18, 2016 •

edited

Edited 1 time

lschmierer edited May 18, 2016 (most recent)

Aceeri commented May 20, 2016 •

edited

Edited 1 time

Aceeri edited May 20, 2016 (most recent)

Aceeri commented Jun 1, 2016 •

edited

Edited 1 time

Aceeri edited Jun 1, 2016 (most recent)

kvark reviewed Jun 7, 2016

View changes

kvark reviewed Jun 10, 2016

View changes

kvark reviewed Jun 10, 2016

View changes

Aceeri Jun 11, 2016 •

edited

Edited 1 time

Aceeri edited Jun 11, 2016 (most recent)

kvark reviewed Jun 10, 2016

View changes

kvark reviewed Jun 10, 2016

View changes

kvark merged commit `34a746a` into amethyst:develop Jun 15, 2016
1 check passed