QEDK / configparser-rs

A simple configuration parsing utility with no dependencies built on Rust.
https://crates.io/crates/configparser
Other
66 stars 21 forks source link

Ini::load should accept a Path instead of str #16

Closed mtorromeo closed 3 years ago

mtorromeo commented 3 years ago

Hi, I find it odd that I have to pass an str to Ini::load when I already have a Path, only for configparser to convert the str back to a Path.

While in practice I can work around the limitation, this might create problems in theory since not every Path can be converted to str.

QEDK commented 3 years ago

Hi, I find it odd that I have to pass an str to Ini::load when I already have a Path, only for configparser to convert the str back to a Path.

This was intended actually, since we wanted to emulate the behaviour of the Python standard library. The problem is that we couldn't have path-like objects similar to Python in Rust due to strict type checking, which meant only using strings.

While in practice I can work around the limitation, this might create problems in theory since not every Path can be converted to str.

At the outset, this seems relatively easy to solve, which is to just add another function like load_from_path(). This should also alleviate your mentioned use-case. Let me know if you have something more clever.

mtorromeo commented 3 years ago

👍🏻 a new method should be fine and it's the only solution if you want to maintain backward compatibility.

super-continent commented 3 years ago

Why not use AsRef\?

QEDK commented 3 years ago

Why not use AsRef?

I'm not a Rust prodigy, so please bear with me. I looked in the documentation and it does seem to the be the idea behind path-like objects, so that's great. Could you give me a more exact idea on how you would want to implement this?

super-continent commented 3 years ago

It's actually quite simple, you would want to make the function take a generic argument with a trait constraint that says it must be able to get used as though it were a reference to a Path.

The function would look something like this:

fn<T: AsRef<Path>>(path: T) {
    ...use path here...
}
QEDK commented 3 years ago

It's actually quite simple, you would want to make the function take a generic argument with a trait constraint that says it must be able to get used as though it were a reference to a Path.

The function would look something like this:

fn<T: AsRef<Path>>(path: T) {
    ...use path here...
}

Figured that out after a while, had a lively discussion regarding strategies on Discord as well, thanks for the help! @mtorromeo @super-continent this has been implemented in the latest commit, feel free to check it out (comment here if there are still remaining issues!)