Added read file str #276
Added read file str #276
Conversation
|
How about defaulting |
Definitely legit. |
fs/read_file_str.ts
Outdated
| * Read file synchronously and output it as a string. | ||
| * | ||
| * @param filename File to read | ||
| * @param encoding Encoding of the file. Default = "utf-8" |
There was a problem hiding this comment.
JSDoc supports specifying defaults:
| * @param encoding Encoding of the file. Default = "utf-8" | |
| * @param [encoding="utf-8"] Encoding of the file. |
However, since TextDecoder defaults to ITF-8, should this just pass on undefined?
There was a problem hiding this comment.
JSDoc supports defaults Without the type definition? Thought it was needed due to my linter. My bad.
About the undefined value, it may be better to have the default in the jsdoc instead of using implicit default for automatic API documentation. Don't you think?
| * @param filename File to read | ||
| * @param [encoding="utf-8"] Encoding of the file | ||
| */ | ||
| export async function readFileStr( |
There was a problem hiding this comment.
how about
| export async function readFileStr( | |
| export async function readFile( |
There was a problem hiding this comment.
May be confusing with the Deno.readFile no?
There was a problem hiding this comment.
@zekth
fs module is used to add some quick and easy methods
Returning a string is the most common method for developer
In most cases, Deno.readFile returns the Reader is not commonly used
So I don’t think these conflicts.
There was a problem hiding this comment.
Totally agree with you. First it was named like this to be in Deno and not in fs-extra as you can see in the ref.
fs/read_file_str.ts
Outdated
| */ | ||
| export async function readFileStr( | ||
| filename: string, | ||
| encoding: string = "utf-8" |
There was a problem hiding this comment.
encoding should be an options object in case of other fields in the future. eg flag
| encoding: string = "utf-8" | |
| options: ReadOptions = {} |
|
Rebased and done the reviews @ry |
|
Since there is a read interface, should there be a write interface? |
As discussed in : denoland/deno#1925
Moving
readFileStr/readFileStrSynctofs-extra