Skip to content

Centralise error positioning + cover custom de errors#356

Merged
kvark merged 5 commits into
ron-rs:masterfrom
juntyr:203-error-positions
Feb 19, 2022
Merged

Centralise error positioning + cover custom de errors#356
kvark merged 5 commits into
ron-rs:masterfrom
juntyr:203-error-positions

Conversation

@juntyr

@juntyr juntyr commented Dec 30, 2021

Copy link
Copy Markdown
Member

Fixes #203 and supersedes #209. Also includes some small minor refactoring of the ErrorCode names.

In particular, the Deserializer now uses ErrorCode as its error type, no longer having to pretend to produce error positions in all cases (which as #209 showed were missing in all serde-internal and user-generated cases, and can only be added during deserialization to the former). Instead, the exposed helper functions from_str etc. now add the positional information afterwards, which cleans up the code and gives us positions for custom errors in serde-code and user-code for free (though the position still only points at the character after the error or directly at it).

  • I've included my change in CHANGELOG.md

@juntyr juntyr force-pushed the 203-error-positions branch from 7d411bc to 1889d64 Compare December 30, 2021 09:54
@juntyr juntyr marked this pull request as ready for review December 30, 2021 10:02
@juntyr

juntyr commented Dec 30, 2021

Copy link
Copy Markdown
Member Author

?r @torkleyy @kvark

@juntyr juntyr requested review from kvark and torkleyy December 30, 2021 10:02
@juntyr

juntyr commented Dec 30, 2021

Copy link
Copy Markdown
Member Author

There is also the question whether ron should split the error type into two, one for ser and one for de, or keep the combined one (like serde_json). What are your thoughts? For ser it seems like we only generate Io and Message errors that are never positioned anyways (unless we wanted to arbitrarily track a fake line and column somehow).

@juntyr juntyr force-pushed the 203-error-positions branch from c9fe4b8 to 6ce56de Compare December 30, 2021 18:28
@juntyr

juntyr commented Jan 26, 2022

Copy link
Copy Markdown
Member Author

?r @torkleyy Any thoughts?

@juntyr juntyr force-pushed the 203-error-positions branch from 6ce56de to f8c2b51 Compare January 28, 2022 17:36
@kvark

kvark commented Feb 18, 2022

Copy link
Copy Markdown
Collaborator

I'm very sorry that neither me or torkley are responsive!
If you are confident with the move, please feel free to commit without reviews if N days passed without any feedback (as long as this deadline is visible). For this PR, I'll review it now.

Comment thread src/de/id.rs Outdated
@juntyr juntyr force-pushed the 203-error-positions branch from ed3253a to 648fc5e Compare February 19, 2022 05:20
@kvark kvark merged commit e542eff into ron-rs:master Feb 19, 2022
@juntyr juntyr deleted the 203-error-positions branch February 19, 2022 22:03
@torkleyy torkleyy mentioned this pull request Jun 6, 2022
2 tasks
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.

Some errors are missing line number information.

2 participants