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

Improve documentation for file dialogs #5171

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

ErikKalkoken
Copy link
Contributor

Description:

When using file dialogs it is currently unclear from the documentation whether the reader/writer passed in the callbacks need to be closed. This may lead to obscure bugs, since the file is often still read/created. I therefore like to propose extending the comments to clarify that those in fact must be closed by the callback.

I further like to propose adding variable names in the callbacks to make it clearer to the user what they are (e.g. those will be added to the users code though auto-complete).

See also related conversation with Andy on Discord.

Checklist:

  • Tests included. (N/A)
  • Lint and formatter run with no errors.
  • Tests all pass.

Where applicable:

N/A

@coveralls
Copy link

coveralls commented Sep 29, 2024

Coverage Status

coverage: 66.042% (+0.02%) from 66.027%
when pulling a9b105e on ErikKalkoken:improve-file-docs
into 119d2f8 on fyne-io:develop.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Thanks for this. Probably important to note that the reader should only be closed when it is not nil - perhaps changing the order of the last two lines in each block?

Also please fix the grammar of the last one - the comma is not wanted in that position.

@ErikKalkoken
Copy link
Contributor Author

Thanks for this. Probably important to note that the reader should only be closed when it is not nil - perhaps changing the order of the last two lines in each block?

Also please fix the grammar of the last one - the comma is not wanted in that position.

Thanks for your feedback. I incorporated both of your points.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Many thanks

@Jacalz Jacalz merged commit 3f50cad into fyne-io:develop Sep 30, 2024
12 checks passed
@ErikKalkoken ErikKalkoken deleted the improve-file-docs branch September 30, 2024 23:49
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.

4 participants