Skip to content

extract linewriter into a context manager #1696

Draft
graingert wants to merge 6 commits intojazzband:mainfrom
graingert:extract-linewriter-into-a-context-manager
Draft

extract linewriter into a context manager #1696
graingert wants to merge 6 commits intojazzband:mainfrom
graingert:extract-linewriter-into-a-context-manager

Conversation

@graingert
Copy link
Member

@graingert graingert commented Oct 7, 2022

#1695 is needed for the multiline cmgr expression here:

        cmgr: (
            contextlib.AbstractContextManager[_DryRunWriter]
            | contextlib.AbstractContextManager[_LineWriter]
        ) = (
            contextlib.nullcontext(_DryRunWriter())
            if self.dry_run
            else _LineWriter.create(buffer=self.dst_file, newline=self.linesep)
        )
Contributor checklist
  • Provided the tests for the changes.
  • Assure PR title is short, clear, and good to be included in the user-oriented changelog
Maintainer checklist
  • Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@graingert graingert added the skip-changelog Avoid listing in changelog label Oct 7, 2022
@graingert
Copy link
Member Author

the actual change for this PR is here 55bcc80 (#1696)

@graingert
Copy link
Member Author

closing in favour of #1698

@graingert graingert closed this Oct 7, 2022
@graingert graingert deleted the extract-linewriter-into-a-context-manager branch October 7, 2022 13:44
@graingert graingert restored the extract-linewriter-into-a-context-manager branch November 9, 2022 10:20
@graingert graingert reopened this Nov 9, 2022
sirosen added a commit to sirosen/pip-tools that referenced this pull request Feb 12, 2026
To abstract away the distinction between writing lines in dry-run mode
and non-dry-run mode, define a small protocol for a "line writer" which
takes and emits lines of text as `str`s.

This idea originates in jazzband#1696 , but was not completed at the time. On
review, the change is perfectly sound and can be implemented with some
slightly clearer types using a protocol to define the interface common to
the two codepaths.
The abstraction improves readability, especially by moving a `finally`
block into a contextmanager `__exit__` function.

Co-authored-by: Thomas Grainger <413772+graingert@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments