rage icon indicating copy to clipboard operation
rage copied to clipboard

age: Remove `Identity::unwrap_stanza`

Open str4d opened this issue 1 year ago • 1 comments
trafficstars

Per https://github.com/str4d/rage/pull/507#discussion_r1703367459, I'm going to remove Identity::unwrap_stanza, and make Identity::unwrap_stanzas a required method. The latter will also be documented as always receiving all stanzas from the same file (rather than the current documentation that this is assumed).

str4d avatar Aug 04 '24 19:08 str4d

Hmm, after deciding to do this, I took another look at @FiloSottile's age and realised that although its Identity only exposes multi-stanza unwrapping, every single impl uses an internal multiUnwrap helper that does exactly what the default impl of Identity::unwrap_stanzas does: https://github.com/str4d/rage/blob/a510e76bf8b5ba1c31d873c38fd1b8056091538f/age/src/lib.rs#L216-L218

The Rust impl is significantly cleaner because Rust has Option and can return Option<Result<_, _>>, whereas Go matches on the returned err looking for a sentinel ErrIncorrectIdentity that serves the same role as None in our API.

So, I'm now back on the fence about whether to remove Identity::unwrap_stanza. It's currently the case that Decryptor only ever calls Identity::unwrap_stanzas, so we could just document on Identity that its canonical entry point is Identity::unwrap_stanzas, and then identity impls that want to add file-level stanza checks can do so by overriding the default impl.

str4d avatar Aug 04 '24 21:08 str4d