Skip to content

Update get_mut() documentation to make clear how 'inner` can be used - #558

Merged
Byron merged 2 commits into
mainfrom
lib-doc-update
Aug 26, 2026
Merged

Update get_mut() documentation to make clear how 'inner` can be used#558
Byron merged 2 commits into
mainfrom
lib-doc-update

Conversation

@Byron

@Byron Byron commented Jul 28, 2026

Copy link
Copy Markdown
Member

This should help steering towards reset() in places where flush() + swap(get_mut, x) might otherwise be used.

It also mentions how to use get_mut() for streaming.

Fixes #555

@Byron Byron changed the title Update get_mut() documentation to make clear how 'inner` can be used. Update get_mut() documentation to make clear how 'inner` can be used Jul 28, 2026
@Byron
Byron requested a review from jongiddy July 28, 2026 12:13
Comment thread src/deflate/write.rs Outdated
Comment on lines +53 to +59
/// The underlying writer may be mutated or replaced as long as this
/// preserves the bytes and ordering of the logical output stream.
///
/// For streaming output, call [`flush`](Write::flush) before replacing the
/// writer, such as with [`std::mem::take`], to retrieve all output produced
/// so far. Concatenate output from each writer to reconstruct the complete
/// stream.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This perhaps over-emphasizes the need for flushing. You can replace a Write implementation at any time without affecting the state of the Encoder/Decoder. To obtain the full output you must (at least logically) concatenate the output of all writers in order, possibly requiring a flush on each of them. However, it is not required to flush the Encoder/Decoder before each replacement.

Maybe something more like:

Suggested change
/// The underlying writer may be mutated or replaced as long as this
/// preserves the bytes and ordering of the logical output stream.
///
/// For streaming output, call [`flush`](Write::flush) before replacing the
/// writer, such as with [`std::mem::take`], to retrieve all output produced
/// so far. Concatenate output from each writer to reconstruct the complete
/// stream.
/// The underlying writer may be mutated or replaced as long as this
/// preserves the bytes and ordering of the logical output stream.
/// Concatenate output from each writer to reconstruct the complete
/// stream.
///
/// To ensure output is retrieved as soon as it is available, call [`flush`](Write::flush)
/// before replacing the writer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The part that neither of these descriptions really express is that calling flush changes the output bitstream by inserting a sync-flush. That's necessary to ensure that all input bytes passed in prior to the flush can be decoded without needing further bytes of output produced by any calls following the flush.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jongiddy and @fintelia! Particularly the "flush has side-effects" note is something the caller should definitely be aware of.

What about this?

Encoders
    /// Acquires a mutable reference to the underlying writer.
    ///
    /// The underlying writer may be mutated or replaced as long as this
    /// preserves the bytes and ordering of the logical output stream.
    /// Concatenate output from each writer to reconstruct the complete stream.
    ///
    /// Replacing the writer does not require [`flush`](Write::flush). Call it
    /// first when all input accepted so far must be decodable without output
    /// from later writes. This inserts a sync-flush point and changes the output
    /// bitstream. This is useful before applying [`std::mem::take`] to
    /// [`get_mut`](Self::get_mut) when forwarding the stream incrementally.
    ///
    /// To start a new stream, use [`reset`](Self::reset); replacing the writer
    /// does not reset this encoder.
Decoders
    /// Acquires a mutable reference to the underlying writer.
    ///
    /// The underlying writer may be mutated or replaced as long as this
    /// preserves the bytes and ordering of the logical output stream.
    /// Concatenate output from each writer to reconstruct the complete stream.
    ///
    /// Replacing the writer does not require [`flush`](Write::flush). Call it
    /// first to write all decompressed output currently available to the
    /// current writer. This is useful before applying [`std::mem::take`] to
    /// [`get_mut`](Self::get_mut) when forwarding output incrementally.
    ///
    /// To start a new stream, call [`finish`](Self::finish) and create a new
    /// decoder; replacing the writer does not reset it.

I have also updated the PR to reflect the change everywhere, with both forms.

@Byron Byron added the question label Aug 5, 2026
Byron and others added 2 commits August 5, 2026 14:50
#555)

This should help steering towards `reset()` in places where
`flush() + swap(get_mut, x)` might otherwise be used.

Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <codex@openai.com>
Assisted-by: GPT 5.6
Co-authored-by: jongiddy <jongiddy@gmail.com>
Co-authored-by: Jonathan Behrens <fintelia@gmail.com>
Co-authored-by: GPT 5.6 <codex@openai.com>
@Byron Byron removed the question label Aug 26, 2026
@Byron
Byron merged commit ed93d4f into main Aug 26, 2026
29 checks passed
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.

Confusing warning on

3 participants