crissdev / gulp-yaml

A Gulp plugin to convert YAML to JSON
MIT License
24 stars 7 forks source link

fix #12 #13

Closed zz5840 closed 6 years ago

zz5840 commented 6 years ago

thanks for helping

ehmicky commented 6 years ago

@crissdev @zz5840 It seems to me that with this fix, YAML files whose content is only a single null, !!js/undefined or false value will not be parsed correctly.

zz5840 commented 6 years ago

@ehmicky how about change https://github.com/crissdev/gulp-yaml/pull/13/files#diff-168726dbe96b3ce427e7fedce31bb0bcR25 to file.contents = result !== undefined ? result : ""

ehmicky commented 6 years ago

I am not sure whether file.contents should be set to an empty string if an error arises. Also this does not account for when the YAML document is only the value !!js/undefined.

zz5840 commented 6 years ago

@ehmicky Oh sorry, file.contents should be a Buffer. I just want to fix the bug that gulp-plumber is invalid when an error occurs and it do affect.

ehmicky commented 6 years ago

Just to be precise for fixing this issue: file.contents can actually be a Buffer, a Stream or null.

zz5840 commented 6 years ago

@ehmicky I'm sorry for my careless, I just want to say file.contents should not be a string, so the code above are wrong

crissdev commented 6 years ago

@ehmicky, Proposed changes are in empty-content-on-error branch - An empty string is returned in case of an error. Please have a look at it if you have time. Thanks.

@zz5840 I tried to reproduce #12 but without luck. The project I used to test it is available here https://github.com/crissdev/gulp-yaml-empty-content-on-error.

ehmicky commented 6 years ago

@crissdev Just did! I added a comment here: https://github.com/crissdev/gulp-yaml/commit/012605d093a4c27bba84b2114cc988bc3891e2d9#comments

crissdev commented 6 years ago

Thanks for your prompt response @ehmicky