-
Notifications
You must be signed in to change notification settings - Fork 125
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
- added possibility to write comment tags #87
Conversation
- added possibility to write comment tags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @brainbugfix! add a round-trip test for this?
types will also need to be updated
lib/encoder.js
Outdated
|
||
function writeCOM(comments) | ||
{ | ||
if (typeof comments === "undefined") return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (typeof comments === "undefined") return; | |
if (typeof comments !== 'string') return; |
perhaps? but ya know, inside the loop with an array check here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not work, I am afraid. The type is 'object' as it is an array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, misunderstood your comment. I will change that tomorrow.
- added checks for comments being an array and the content is string
thanks that change looks great, would you mind tackling...
:D |
I am not sure if I understand what you want me to do. I can add a test, that should not be a big deal but what exactly do you mean with updating the types? |
Oh sorry, I mean update the typescript definitions in Line 13 in 417e8e2
It's already a bit out of date but just changing the first encode parameter to |
- updated the typescript - added a test for writing the comments
Did what you asked me for. Can you please double-check the TypeScript because I've never used this before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @brainbugfix this is great!
if all these suggestions are ok with you @brainbugfix I'm ready to merge! :) |
thanks again :) |
I added the possibility to add the comments to the image during encoding as they come from decoding or added manually