Position is actually not optional

[fraillt]

Hello,

While investigating in which cases it is safe to unwrap position , I came to conclusion that it is actually always present.
There's exactly two places that it was None before:

• while creating ByteRecord - now it defaults to Position::new() which totally makes sense
• while returning ErrorKind::UnequalLengths from writer - I wrote a comment on a type, that for writer position is not tracked.

Even though it makes things easier to use and reason about (no need to unwrap anything, or handle None case that will never happen), this is however breaking change for the end user...

[BurntSushi]

This is a plausible choice to make in the design of this crate, but I felt it was more important to represent absence of a position rather than lie about it if the position isn't available or hasn't been set yet. The alternative isn't really this PR IMO, but rather, to make construction of a record require a Position provided by the caller. But that's somewhat annoying too, and not always relevant.

But even separate from the design question, even if I conceded this PR was 100% unambiguously better, there's no way I'm putting out a csv 2 just for this change. In general, you should discuss breaking changes before submitting PRs for them. Especially for projects with a non-zero major version.

I'd suggest opening an issue about this and talking about the trade offs. I can consider it if and when a csv 2 is ever released.

[fraillt]

Everything is fine, I didn't expected it to land either, I just wanted to bring attention about it :)
Sometimes it makes things easier to discuss when you can evaluate work needed and what exactly is affected in advance.
Just in case, it's not clear, it was mostly compiler-driven-development, so it's really low effort PR.

Regarding creating an issue I'll leave it up to you :)

If you already have a list of "nice-to-have" breaking changes for csv 2, I would strongly suggest to remove optional from position as well, as it makes user code a lot cleaner and simpler (there are code bases, that doesn't allow to unwrap things, so you can imagine the burden on end users to always handle situation, that never happens.)

In my opinion, this PR also suggest that splitting ErrorKind (or part of it) for writer and reader would also be "nice-to-have", if you don't want to "lie about it if the position isn't available".

[BurntSushi]

Aye understood. Makes sense. I appreciate it.

there are code bases, that doesn't allow to unwrap things

Yeah, I don't really think this is good practice. And I don't ever plan to spend any effort making this sort of thing easier or nicer. See: https://blog.burntsushi.net/unwrap/