Skip to content

Generate helpful errors from dotenv! macro#53

Closed
Plecra wants to merge 2 commits into
dotenv-rs:masterfrom
Plecra:pretty-compile-error
Closed

Generate helpful errors from dotenv! macro#53
Plecra wants to merge 2 commits into
dotenv-rs:masterfrom
Plecra:pretty-compile-error

Conversation

@Plecra

@Plecra Plecra commented Jun 25, 2020

Copy link
Copy Markdown
Contributor

An implementation for #9

@codecov

codecov Bot commented Jun 25, 2020

Copy link
Copy Markdown

Codecov Report

Merging #53 into master will decrease coverage by 2.43%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
- Coverage   86.77%   84.33%   -2.44%     
==========================================
  Files           7        7              
  Lines         242      249       +7     
==========================================
  Hits          210      210              
- Misses         32       39       +7     
Impacted Files Coverage Δ
dotenv_codegen_implementation/src/lib.rs 0.00% <0.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 8c2ba38...31e48e8. Read the comment docs.

@Plecra

Plecra commented Jun 25, 2020

Copy link
Copy Markdown
Contributor Author

What would you like me to do about those regressions? I'm pretty sure I haven't removed any tests, and the only changes I made were in the macro implementation

@ghost

ghost commented Jun 25, 2020

Copy link
Copy Markdown

I'll look into it, but its been long enough since code coverage was run, I wouldn't be surprised if its just something going wrong with the codecov tool

@Plecra

Plecra commented Jun 26, 2020

Copy link
Copy Markdown
Contributor Author

Ooh, I can actually understand the report now. It's identified the couple error cases that I introduced but didn't test. I don't think it's worth putting together a compile error test harness for those couple cases though...

@ghost

ghost commented Jun 26, 2020

Copy link
Copy Markdown

@Plecra its also in codegen, which can't really be code coverage tested.

Err(VarError::NotPresent) | Err(VarError::NotUnicode(_)) => panic!("{}", err_msg),
match env::var(var_name.value()) {
Ok(val) => Ok(quote!(#val).into()),
Err(VarError::NotPresent) | Err(VarError::NotUnicode(_)) => Err(syn::Error::new(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we should break up this match arm now that we're producing more descriptive error messages. NotUnicode doesn't match with environment variable {} not defined

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Taking a second look at this, I want to mirror the behavior of std::env!, which actually doesn't differentiate between the two errors. Not that that means we can't though

@Plecra

Plecra commented Jul 14, 2020

Copy link
Copy Markdown
Contributor Author

Continued in #58

@Plecra Plecra closed this Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant