refactor: introduce ProblemReport._write_base64_encoded_line

[bdrung]

Introduce ProblemReport._write_base64_encoded_line which writes one line of a RFC822 multiline. This will write blocks line by line instead of writing the leading space after writing a line.

[codecov]

Codecov Report

Merging #238 (9f853d3) into main (5d900fa) will increase coverage by 10.52%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main     #238       +/-   ##
===========================================
+ Coverage   73.16%   83.69%   +10.52%     
===========================================
  Files          91       93        +2     
  Lines       18031    18756      +725     
===========================================
+ Hits        13193    15698     +2505     
+ Misses       4838     3058     -1780     

Files	Coverage Δ
problem_report.py	97.27% <100.00%> (-0.01%)	⬇️

... and 28 files with indirect coverage changes

[schopin-pro]

I don't think you put the cursor far enough in the refactoring. The new method is still a footgun, as it requires the caller to actually prepare the terrain first by writing out the RFC822 key and base64 header, which means the base64-specific knowledge is split in two places.

Instead, I suggest having something like this:

def _write_base64_paragraph(file, typing.BinaryIO, key: str, data: list[bytes]):
   """Write the data out in a RFC 822 base64 multiline paragraph, using one line per item in the data list.
   """

This would even make the control flow a bit easier in _write_key_and_binary_value_compressed_and_encoded_to_file ^{_{(wow that name is a handful, typing it out takes forever)}}. You don't have to rewind the file in case of aborted write, you just never call the helper method :)

[bdrung]

issue: _write_base64_paragraph requires to construct a complete data list containing the whole data. This data structure can become quite large, because it can contain the coredump.

[schopin-pro]

Meh, that's barely relevant 😉.

Maybe have

def _binary_chunks_from_value(v: Any) -> Generator[bytes, None, None]:
    pass

def _write_base64_paragraph(file, typing.BinaryIO, key: str, data: Iterable[bytes]):
    pass

def _write_key_and_binary_value_compressed_and_encoded_to_file(
    self, file: typing.BinaryIO, key: str
) -> None:
    _write_base64_paragraph(key, file, _binary_chunks_from_value(self.data[key]))

With binary_chunks_from_value being essentially the same as your current _write_key_and_binary_value_compressed_and_encoded_to_file with calls to yield instead of calls to write_base64_encoded_line and the base64 header + recovery code moved into an except block in _write_base64_paragraph.

WDYT?

[bdrung]

That idea sounds promising. Do you want to implement it or should I give it a try?

[schopin-pro]

I'll give it a shot.

[bdrung]

Closing this PR in favor of #242.