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.

Sign up for GitHub

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

feat: read config toml file #7

Merged
merged 9 commits into from
Jan 3, 2023
Merged

feat: read config toml file #7

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
34663c4
add serde to serialize toml file
1garo Oct 11, 2022
e735187
initial idea on how to read toml file
1garo Oct 11, 2022
5e9ec05
update typo on readme
1garo Oct 11, 2022
65c8aa1
first letter uppercase to match rust enum
1garo Oct 11, 2022
461d008
feat: WIP but some improvements
1garo Oct 13, 2022
44cdb05
load from path to improve test implementation
1garo Oct 14, 2022
86f491f
add dirs as a dependency
1garo Oct 14, 2022
2625173
Merge branch 'main' into feat/read-config-toml-file
raphamorim Oct 15, 2022
8fc4786
Merge branch 'main' into feat/read-config-toml-file
raphamorim Jan 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ The configuration should be the following paths otherwise Rio will use the defau
```toml
# Rio configuration file

# <perfomance> Set WGPU rendering perfomance
# default: high
# options: high, average, low
perfomance = "high"
# <performance> Set WGPU rendering perfomance
# default: High
# options: High, Average, Low
performance = "High"

## TODO: Add more configs
```
Expand Down Expand Up @@ -67,4 +67,4 @@ perfomance = "high"

## Credits

- Text mod code is from with https://github.com/hecrj/wgpu_glyph
- Text mod code is from with https://github.com/hecrj/wgpu_glyph
4 changes: 3 additions & 1 deletion config/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ version = "0.1.0"
edition = "2021"

[dependencies]
toml = "0.5"
toml = "0.5"
serde = { version = "1.0", features = ["derive"] }

53 changes: 48 additions & 5 deletions config/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,57 @@
pub fn add(left: usize, right: usize) -> usize {
left + right
use serde::Deserialize;

#[derive(Debug, Deserialize, PartialEq)]
enum Performances {
High,
Average,
Low,
}

#[derive(Debug, Deserialize, PartialEq)]
pub struct Config {
performance: Performances,
width: Option<u16>,
Copy link
Owner

Choose a reason for hiding this comment

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

Believe is better have as u16 instead of Option. To avoid additional unwrap when using the configuration struct later. Since all properties should have a default we can unwrap it on the new.

Also will be good to have a default in the impl. E.g: https://doc.rust-lang.org/std/default/trait.Default.html

Copy link
Contributor Author

@1garo 1garo Oct 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I have started the implementation here

I couldn't come with a solution for when the user have the config file, but didn't pass one of the parameters.

Because if I set the struct as below:

struct Config {
    performance: Option<Performance>,
    width: u16,
    height: u16,
}

Now the problem is that when we parse, as the user did not specified performance, it is None,

Config {
    performance: None,
    width: 300,
    height: 200,
}

Did you have something in mind?

obs: the code can be not that rusty, still learning some patterns, if you think something can improve, just let me know

Copy link
Owner

Choose a reason for hiding this comment

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

No worries at all @1garo, you are doing a great job. Usually when you're wrapping and come as None, you can use unwrap_or_else(|| to return the default. Let me know if you still have questions, we can book a day and do this part together 👍

Copy link
Contributor Author

@1garo 1garo Oct 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

sounds great, if you can talk today or tomorrow, just say where I can reach out to you!

Copy link
Owner

Choose a reason for hiding this comment

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

If you have telegram, just drop me a message on same username as GH. I also have discord just in case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think discord is easier for me, I used telegram once in my life. 1garo#7086

height: Option<u16>,
}

pub fn load() -> Config {
let toml_str = r#"
# Rio configuration file

# <perfomance> Set WGPU rendering perfomance
# default: High
# options: High, Average, Low
performance = "High"

# <height> Set default height
# default: 400
height = 400

# <width> Set default width
# default: 600
width = 600

## TODO: Add more configs
"#;

let decoded: Config = toml::from_str(toml_str).unwrap();

decoded
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn it_works() {
let result = add(2, 2);
assert_eq!(result, 4);
fn load_config() {
let expected = Config {
performance: Performances::High,
width: Some(600),
height: Some(400),
};

let result = load();
assert_eq!(result, expected);
}
}