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.

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

expected error strings always quote what was expected, even if it isn't a literal #334

Closed
epage opened this issue Jan 10, 2022 · 3 comments · Fixed by #335
Closed

expected error strings always quote what was expected, even if it isn't a literal #334

epage opened this issue Jan 10, 2022 · 3 comments · Fixed by #335

Comments

@epage
Copy link
Contributor

epage commented Jan 10, 2022

toml_edit sets some expected for not what character is next but what grammar item is expected (e.g. "key").

This means they are quoted, like literals. See review comments on rust-lang/cargo#10086

@epage
Copy link
Contributor Author

epage commented Jan 10, 2022

I'd propose we have a

enum Item {
    Literal(String),
   Other(String),
}

(obviously some naming could be improved)

We could have some impl Froms to keep the API compatible.

I'm willing to implement this, just looking for a go ahead.

@Marwes
Copy link
Owner

Marwes commented Jan 11, 2022

It would be a breaking change to make this change to the "easy errors" now and I am not willing to do that for just this (the error handling could use an overhaul though). So you may need to implement your own errors, or convert the easy errors before displaying them.

@epage
Copy link
Contributor Author

epage commented Jan 11, 2022

In looking to see if there was a way to do this without a breaking change, it turned out combine already has all the information it needs. In case you weren't referring to the formatting change as a breaking change (since most textual outputs are not considered stable and stability wasn't mentioned for #333), I am preparing a PR with the change. At least within combine's docs, it works exactly as expected!

epage added a commit to epage/combine that referenced this issue Jan 11, 2022
When reviewing cargo moving fron `toml-rs` (hand-rolled parser) to
`toml_edit` (`combine`-based parser), one piece of feedback was about
only wanting tokens quoted and not textual descriptions.  It looks like
`combine` tracks all of the needed information and just a re-formatting
of the output is needed.

Fixes Marwes#334
epage added a commit to epage/combine that referenced this issue Jan 11, 2022
When reviewing cargo moving fron `toml-rs` (hand-rolled parser) to
`toml_edit` (`combine`-based parser), one piece of feedback was about
only wanting tokens quoted and not textual descriptions.  It looks like
`combine` tracks all of the needed information and just a re-formatting
of the output is needed.

Fixes Marwes#334
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 a pull request may close this issue.

2 participants