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

Add writeToFile, writeToFileAsBytes, writeToFileAsString #85

Merged
merged 2 commits into from
Jul 16, 2022

Conversation

jfaltis
Copy link
Contributor

@jfaltis jfaltis commented Jul 14, 2022

This PR implements #61.

@alexrintt alexrintt added the stage:dev This issue/feature doesn't seems to be stable enough to be considered released in the next version. label Jul 15, 2022
result: MethodChannel.Result,
uri: String,
content: ByteArray,
mode: String,
Copy link
Owner

Choose a reason for hiding this comment

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

Syntax error because of this comma:

Warning The feature "trailing commas" is only available since language version 1.4

@@ -281,6 +282,82 @@ Future<DocumentFile?> createFileAsString(
);
}

/// {@template sharedstorage.saf.writeToFile}
/// Convenient method to write to a file using either String or raw bytes.
Copy link
Owner

@alexrintt alexrintt Jul 15, 2022

Choose a reason for hiding this comment

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

Suggested change
/// Convenient method to write to a file using either String or raw bytes.
/// Convenient method to write to a file using either [String] or raw bytes [Uint8List].

You can also apply this change for createFile function.

}

final args = <String, dynamic>{
'uri': uri.toString(),
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
'uri': uri.toString(),
'uri': '$uri',

Comment on lines 330 to 334
var writeMode = 'wt';

if (mode == FileMode.append || mode == FileMode.writeOnlyAppend) {
writeMode = 'wa';
}
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
var writeMode = 'wt';
if (mode == FileMode.append || mode == FileMode.writeOnlyAppend) {
writeMode = 'wa';
}
final writeMode =
mode == FileMode.append || mode == FileMode.writeOnlyAppend ? 'wa' : 'wt';


/// {@template sharedstorage.saf.writeToFileAsString}
/// Convenient method to write to a file.
/// using `content` as String instead Uint8List.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/// using `content` as String instead Uint8List.
/// using `content` as [String] instead [Uint8List].

You can also apply this change for createFileAsString function.


Given the document uri, opens the file in the specified `mode` and writes the `bytes` to it.

`mode` represents the mode in which the file will be opened for writing. Use `FileMode.write` for truncating and `FileMode.append` for appending to the file.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
`mode` represents the mode in which the file will be opened for writing. Use `FileMode.write` for truncating and `FileMode.append` for appending to the file.
`mode` represents the mode in which the file will be opened for writing. Use `FileMode.write` for truncating (overwrite) and `FileMode.append` for appending to the file.

Comment on lines 578 to 579


Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change

);

/// Write to a file using a [Uint8List] as file contents [bytes]
final DocumentFile? createdFile = writeToFile(
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
final DocumentFile? createdFile = writeToFile(
final bool? success = writeToFile(

Guess this function returns a boolean representing the whether or not the operation was successful, if not let me know but I think it was just a typo.

);

/// Append to a file using a [Uint8List] as file contents [bytes]
final DocumentFile? createdFile = writeToFile(
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
final DocumentFile? createdFile = writeToFile(
final bool? success = writeToFile(

Guess this function returns a boolean representing the whether or not the operation was successful, if not let me know but I think it was just a typo.

@alexrintt
Copy link
Owner

Great work, I tested and it works gracefully on Android 10+ (Tested on Samsung A30 and POCO M4 PRO).

I left some comments refering to small fixes related to the API documentation and code formatting. If you think something doesn't make sense just reply on it and lets discuss.

You can also pull this branch: lakscastro:feat/write-to-file-fixes, it contains the fix of all comments of this pull request.

Thanks for the hard work!

@jfaltis
Copy link
Contributor Author

jfaltis commented Jul 16, 2022

I am happy to hear that. I agree with all your changes!

@alexrintt alexrintt merged commit ace3244 into alexrintt:master Jul 16, 2022
@alexrintt alexrintt added stage:stable This issue/feature/resource seems to be stable enough to be considered released in the next version. and removed stage:dev This issue/feature doesn't seems to be stable enough to be considered released in the next version. labels Jul 16, 2022
@alexrintt
Copy link
Owner

You did it! Happy to see your contribution here, I'll release on v0.5.0 today or tomorrow.

Any other requests or suggestions let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage:stable This issue/feature/resource seems to be stable enough to be considered released in the next version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants