More idiomatic code

[jplatte]

Are these clear enough the way they are or should any of the changes have more explanation?

[codecov[bot]]

Codecov Report

Merging #57 into master will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #57   +/-   ##
=======================================
  Coverage   86.77%   86.77%           
=======================================
  Files           7        7           
  Lines         242      242           
=======================================
  Hits          210      210           
  Misses         32       32           

Impacted Files	Coverage Δ
dotenv/src/parse.rs	95.34% <ø> (ø)
dotenv/src/lib.rs	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3c1a77b...a077484. Read the comment docs.

[jplatte]

The alternative to .ok() is interesting! I admit this syntax does seem like more idiomatic Rust, rather than just kinda hiding the must_use annotation on Results. I wonder if users will find the change jarring though... Personally I like it ^^

Do you expect me do anything here, e.g. leave that commit out for now?

[Plecra]

Sorry for being vague: I'm not a maintainer of this repository, so there's no need to follow my recommendations. I am hoping the let _ style sticks though 😀

Btw, there's an equivalent Result::expect_err method you can use in the same way on the other tests (Ooh, you still need to match on the LineParse)

[jplatte]

Yes, the LineParse matching is why I couldn't update under others (I wanted to).

[jplatte]

Ping @ZoeyR

[ZoeyR]

I am without computer and decent internet for a bit. I'll start working on PRs once I have those in place.

[jplatte]

Ping @ZoeyR

[jplatte]

@ZoeyR any chances you could have another look at this soon, or add another maintainer to the dotenv-rs org?

[tshepang]

The alternative to .ok() is interesting! I admit this syntax does seem like more idiomatic Rust, rather than just kinda hiding the must_use annotation on Results. I wonder if users will find the change jarring though... Personally I like it ^^

I find it jarring, and think dotenv.ok() makes it clear enough that failure is acceptable (meaning that .env is more a convenience than a requirement of the resulting executable).

[kaj]

In projects where I don't want random crashes, I use dotenv like this:

    match dotenv() {
        Ok(_) => (),
        Err(ref err) if err.not_found() => (),
        Err(e) => return Err(e.into()),
    }

I would very much prefer to just have:

    dotenv()?;

... but I do not want an error if there is no .env file. However, I desperatley do want an error in case of an imparsable .env file.

[allan2]

@jplatte I just wanted to mention that I merged this into my fork of dotenv called dotenvy. Thanks for the PR!

[hoijui]

@allan2's repo is now considered the new upstream, so just use it instead of this repo.