add replacer code similar to JSON replacer and (configurable) toJSON #392
Conversation
| jjj = JSON.stringify(obj, repl) | ||
| console.log('yyy => ', yyy); console.log('jjj => ', jjj) | ||
| ***/ | ||
| let object = oobject; // keep original for debug |
There was a problem hiding this comment.
not sure if let is good to use in js-yaml. If the code should be compatible with older browsers, var would be preferable. Having a look at dumper.js, var is definitely the thing.
There was a problem hiding this comment.
about // keep original for debug, I wouldn't recommend that as a motive, you can still assign oobject to a value in the debugger if you want to keep it around. However, as pointed out before, chrome optimizes better when function parameters are not overridden.
This line should be:
var object_ = object;| @@ -682,7 +684,31 @@ function detectType(state, object, explicit) { | |||
| // Serializes `object` and writes it to global `result`. | |||
| // Returns true on success, or false on invalid object. | |||
| // | |||
| function writeNode(state, level, object, block, compact, iskey) { | |||
| function writeNode(state, level, oobject, block, compact, iskey) { | |||
There was a problem hiding this comment.
You shouldn't have to change the signature. use object instead of oobejct. oobject reads badly in documentation.
Rather use later:
var object_ = object;Also, because overwriting values of function parameters has a bad influence on chrome (the browser) performance optimization behavior.
| function writeNode(state, level, object, block, compact, iskey) { | ||
| function writeNode(state, level, oobject, block, compact, iskey) { | ||
| // replacement, sjpt | ||
| /** test case: |
There was a problem hiding this comment.
There is a proper test-suite, we should have tests there.
| console.log('yyy => ', yyy); console.log('jjj => ', jjj) | ||
| ***/ | ||
| let object = oobject; // keep original for debug | ||
| if (typeof object === 'object' && object[state.localReplacer]) { |
There was a problem hiding this comment.
add
&& typeof object[state.localReplacer] === 'function'There was a problem hiding this comment.
This:
&& object[state.localReplacer]must be rather:
&& Object.prototype.hasOwnProperty.call(object, state.localReplacer)| ***/ | ||
| let object = oobject; // keep original for debug | ||
| if (typeof object === 'object' && object[state.localReplacer]) { | ||
| object = object[state.localReplacer].apply(object); |
There was a problem hiding this comment.
This is not called correctly:
JSON.stringify() calls toJSON with one parameter:
- if this object is a property value, the property name
- if it is in an array, the index in the array, as a string
- an empty string if JSON.stringify() was directly called on this object
do it like this:
object_ = object[state.localReplacer](key || '');NOTE: we have a problem here, writeNode is not aware of the key it is writing.
Also, not each invocation of writeNode writes a value, some invocations just write keys, see the isKey parameter. If isKey is true, we should never even consider calling any replacer.
| if (typeof object === 'object' && object[state.localReplacer]) { | ||
| object = object[state.localReplacer].apply(object); | ||
| } | ||
| if (state.replacer) { |
There was a problem hiding this comment.
that's not sufficient to match https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter
|
implemented slightly differently in 359b264 |
I apologize if I haven't created this request correctly; still very unused to git.
I have changed the base file lib/js-yaml/dumper,js, but not built any dist/js-yaml.js or similar files.
Also, there are just two test cases which are for the moment included inline in dumper.js.