Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(file): add context manager to File #998

Merged
merged 5 commits into from Apr 12, 2023

Conversation

msnyder5
Copy link
Contributor

@msnyder5 msnyder5 commented Feb 22, 2023

Summary

Add a context manager to nextcord.File. This will automatically call close() on the file when the context manager exits scope.

Also change force_close to be optional default to None so that we can set force_close to True when using context manager if False was not provided to init.

Old Method

file = nextcord.File(filepath)
await channel.send(file=file)
file.close()

New Method (w/ Context Manager)

with nextcord.File(filepath) as file:
    await channel.send(file=file)

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
    • I have run task pyright and fixed the relevant issues.
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed) Edit: This actually isn't a breaking change as you can still use the old syntax.
  • This PR is not a code change (e.g. documentation, README, ...)

Add Context Manager to `nextcord.File`
feat: add context manager for `File`

Add a context manager to `nextcord.File`. Also change `force_close` to be optional default to None so that we can set `force_close` to True when using context manager if `False` was not provided to init.
@msnyder5 msnyder5 marked this pull request as ready for review February 22, 2023 22:42
feat: add context manager for `File`

Add a context manager to `nextcord.File`. Also change `force_close` to be optional default to None so that we can set `force_close` to True when using context manager if `False` was not provided to init.
nextcord/file.py Outdated Show resolved Hide resolved
nextcord/file.py Show resolved Hide resolved
@EmreTech EmreTech added t: enhancement Type: enhancement - new feature or request p: low Priority: low - not important to be worked on s: awaiting review Status: the issue or PR is awaiting reviews labels Feb 22, 2023
Copy link
Member

@ooliver1 ooliver1 left a comment

Choose a reason for hiding this comment

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

Nice first PR, thanks for the explanation!

nextcord/file.py Outdated Show resolved Hide resolved
nextcord/file.py Outdated Show resolved Hide resolved
nextcord/file.py Outdated Show resolved Hide resolved
Copy link
Member

@ooliver1 ooliver1 left a comment

Choose a reason for hiding this comment

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

Did not need to make that approved.

Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com>
Co-authored-by: Oliver Wilkes <oliverwilkes2006@icloud.com>
nextcord/file.py Outdated Show resolved Hide resolved
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com>
@Skelmis Skelmis requested a review from ooliver1 March 29, 2023 11:37
@Skelmis Skelmis requested a review from EmreTech April 9, 2023 12:41
Copy link
Collaborator

@EmreTech EmreTech left a comment

Choose a reason for hiding this comment

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

Not tested.

Copy link
Collaborator

@alentoghostflame alentoghostflame left a comment

Choose a reason for hiding this comment

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

Haven't tested but it LGTM!

@ooliver1 ooliver1 changed the title feat: add context manager for File feat(file): add context manager to File Apr 12, 2023
@ooliver1 ooliver1 merged commit cf7741d into nextcord:master Apr 12, 2023
@ooliver1 ooliver1 removed the s: awaiting review Status: the issue or PR is awaiting reviews label Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: low Priority: low - not important to be worked on t: enhancement Type: enhancement - new feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants