Make ParseErrorKind public and available through ParseError::kind()

[sbrocket]

ParseErrorKind has also been made #[non_exhaustive] to avoid committing
to only the current set of error kinds, i.e. so that it can be expanded
as needed without an SemVer-breaking change.

Thanks for contributing to chrono!

• Have you added yourself and the change to the changelog? (Don't worry
  about adding the PR number)
• If this pull request fixes a bug, does it add a test that verifies that
  we can't reintroduce it?

Fixes #319

[Toover]

I find the PR good but I wonder why there is a notion of "kind" involved whereas a synonymous notion is provided by the "type" system. Can't ParseError just be an enum and all kinds be values? Sorry if that is a noob question.

[sbrocket]

Can't ParseError just be an enum and all kinds be values?

Yes, ParseError could have directly been an enum instead of there being a separate ParseErrorKind enum type. Switching it from a struct to an enum now, however, would be a breaking change.

[Toover]

Switching it from a struct to an enum now, however, would be a breaking change.

Sorry I am still learning Rust... Given that it has been private until now, why would it break? Is it because the ABI of a struct differs from the one of an enum? So the needed memory allocation won't be right with that new version, unless it is recompiled?

[sbrocket]

ParseError is already public, and this change simply adds a new method to that existing type. ParseErrorKind was not previously public, which is why it was possible to make it #[non_exhaustive]. If the ParseErrorKind type was already public, that would be a breaking change. Also, this is not really a venue meant for general Rust questions. I’d recommend checking out the Rust Discord if you’ve got more questions, it’s a good resource: https://discord.com/invite/rust-lang-community

[PumpkinSeed]

Thanks for the contribution, I just run into an issue which solved by this PR, because I can't return ParseResult in one of my functions, because the ParseErrorKind is private and I want to return Err(ParseError(ParseErrorKind::Invalid)).

[sbrocket]

Thanks for the contribution, I just run into an issue which solved by this PR, because I can't return ParseResult in one of my functions, because the ParseErrorKind is private and I want to return Err(ParseError(ParseErrorKind::Invalid)).

Thank you but I don’t think this change will actually help with that usage. This leaves the field inside the tuple struct private, so I don’t think you would be able to construct your own ParseError value.

[sbrocket]

Maintainers, I haven’t resolved the CHANGELOG.md conflict because it isn’t clear if or when this PR will be looked at, and it seemed likely the conflict would reoccur. If you’re ignoring this because of the conflict please let me know and I’ll fix it, otherwise I would appreciate feedback one way or the other. Thanks.

[ritchie46]

This would also be a valuable addition for us. We want to match the minimal possible values from contiguous slices of possible matches. Not having to materialize the string errors would save a lot of allocations.

[Ploppz]

Why is this not yet merged?

[djc]

The previous maintainer left, and other changes have taken priority. Once CI passes I'm happy to review this.

[Ploppz]

Looking at the test results, #[non_exhaustive] seems to be unsupported in Rust 1.32. Does chrono state a MSRV? Is there a reason why tests are using rustc 1.32 - can this be increased to the current stable version?

[djc]

Yes, 1.32 is the current MSRV for the main branch and will be for the next release, so #[non_exhaustive] is not available.

[Ploppz]

Is it an option to implement it like this in the meantime? https://rust-lang.github.io/rfcs/2008-non-exhaustive.html#how-we-do-this-today

[djc]

Yeah, that's probably an okay solution for now. Probably should add a comment with a // TODO: MSRV.

[Ploppz]

Feel free to already review my changes sbrocket#1

[sbrocket]

Done, thanks @Ploppz!

[Ploppz]

Np! I guess it's up to you to either rebase or merge from main to resolve conflicts.

[sbrocket]

I’ll leave it up to the maintainer to handle the changelog conflict, since I have no idea when this might be reviewed and would otherwise need to repeatedly resolve conflicts there.

[djc]

Please just rebase this and fix the merge conflict. I don't have time to resolve conflicts for every PR -- I will review this pretty soon once it passes CI.

[sbrocket]

Ok, rebased (once; hopefully it makes sense that expecting all authors of pending PRs to rebase their PRs every time the changelog is updated by another merged PR is not a very smooth contribution process). CI isn't going to run automatically because of the first-time contributor approval limitation.

[Ploppz]

I hope it's not out of place with a gentle reminder. Additional question: can this be included in the next version / what's the timeline for the next version release?

[djc]

It should be in the next release. That's mainly waiting for the work in #677 and related PRs. Rest assured that this is still in my GitHub notifications queue and won't be forgotten (that is, no need to remind me again).