EdJoPaTo / website-stalker

Track changes on websites via git
GNU Lesser General Public License v2.1
54 stars 6 forks source link

Current main can't parse it's own example config #170

Closed Teufelchen1 closed 1 year ago

Teufelchen1 commented 1 year ago

[disclaimer: I'm inexperienced in rust]

The current main can not parse the config generated by the current main when using either init or example-config.

Versions

To Reproduce Steps to reproduce the behavior:

  1. Freshly clone the repo
  2. cargo build
  3. cargo run example-config > website-stalker.yaml
  4. cargo run check
  5. See error
    unknown variant `article`, expected one of `css_remove`, `css_select`, `html_markdownify`, `html_prettify`, `html_sanitize`, `html_textify`, `html_url_canonicalize`, `json_prettify`, `regex_replace`, `rss`

Expected behavior

Successful parsing of the config.

Additional information

Beside this, the real bug might be hidden in config.rs, where the unit tests are inadequate. For example this:

#[test]
fn can_parse_example() {
    let conf_str: String = Config::example_yaml_string();
    let _conf: Config = serde_yaml::from_str(&conf_str).unwrap();
}

would have spotted this issue immediately.

EdJoPaTo commented 1 year ago

Thanks for reporting!

Due to a bug in serde_yaml the parsing fails. I improved the test as you suggested and rolled back to serde_yaml 0.8 in 6cb2a6d.

Once the bug is fixed serde_yaml can be upgraded again. The current website-stalker 0.19.0 version is still at serde_yaml 0.8 so it does not need a bugfix release.

Personally I am not that sure about the !Tag syntax serde_yaml 0.9 introduced. It does not seem as beginner friendly as the 0.8 syntax. So staying with 0.8 is also another good idea for usability. (But its not for missing fixes only the latest version will get.)

As a test was added to prevent this in the future and the bug is already reported in the library I will close this issue. Thanks again for reporting!