Add(concat): prefix and suffix options (fix #48) #57
Conversation
| @@ -0,0 +1,3 @@ | |||
| { | |||
There was a problem hiding this comment.
The .vscode folder should not be pushed to git. Remove this & add it to the .gitignore in the root directory.
| * | ||
| * suffix and prefix features | ||
| * @param suffix || null @param prefix || null | ||
| * appending additional suffix and prefix to each data. |
There was a problem hiding this comment.
This JSDoc comment is incorrect:
- Each param definition should be on its own line
- Description should only be before the param comments start
There was a problem hiding this comment.
So if i add the comment like:
/**
* options assign
* @param suffix adding string at the end of the each file
* @param prefix adding string at the beginning of the first packet
*/
var opts = Object.assign({}, ctx.args[0] || {})There was a problem hiding this comment.
The opts i will fix it
| var opts = Object.assign({}, ctx.args[0] || {}) | ||
|
|
||
| // suffix | ||
| if (opts.suffix) { |
There was a problem hiding this comment.
Prefix should only be added if this data packet is the first packet (which it will not always be).
Check if it is the first data piece before adding a prefix.
There was a problem hiding this comment.
How do i know how many packets are coming, can i modified the ctx which will store a count?
There was a problem hiding this comment.
There's no way to know how many packets but you don't need a count. You just need to know what the first one is & what the last one is.
There was a problem hiding this comment.
I feel stupid, which props i can check is the first or the last?
| /** | ||
| * options assign | ||
| */ | ||
| var opts = Object.assign({}, ctx.args[0] || {}) |
There was a problem hiding this comment.
This is only necessary if you are going to modify the opts object, which you aren't. Since you are not modifying the opts, you can just do this: const opts = ctx.args[0]
Also avoid using var in ES2015 code. Use let or const.
There was a problem hiding this comment.
If i changed to javascript const opts=ctx.args[0] would be an error of suffix undefined. so i changed javascript const opts =ctx.args[0] || {}
| // suffix | ||
| if (opts.suffix) { | ||
| data.size += opts.suffix.length | ||
| data.body = Buffer.from(opts.suffix + data.body.toString()) |
There was a problem hiding this comment.
Providing a Buffer is unnecessary. You can just provide a string like this:
data.body = opts.suffix + data.body.toString()
| data.body = Buffer.from(opts.suffix + data.body.toString()) | ||
| } | ||
|
|
||
| // prefix |
There was a problem hiding this comment.
Prefix should be before data and suffix should be after.
| } | ||
|
|
||
| // prefix | ||
| if (opts.prefix) { |
There was a problem hiding this comment.
Suffix should only be added if data.end is true (i.e. it is the last piece of data)
karimsa
changed the title
Codecov Report
@@ Coverage Diff @@
## master #57 +/- ##
=========================================
- Coverage 7.57% 7.53% -0.04%
=========================================
Files 61 61
Lines 2692 2706 +14
=========================================
Hits 204 204
- Misses 2488 2502 +14
Continue to review full report at Codecov.
|
karimsa
changed the title
|
So what am i fixing next? @karimsa |
Relevant issues & PRs
See #48.
Checklist