adding parseStringSync + simple tests #241
Conversation
| @@ -156,11 +156,30 @@ module.exports = | |||
| equ r.sample.$$[10].$$[2]._, '3' | |||
| equ r.sample.$$[10].$$[3]['#name'], 'one' | |||
| equ r.sample.$$[10].$$[3]._, '4' | |||
| equ r.sample.$$[10].$$[4]['#name'], 'two' | |||
| equ r.sample.$$[10].$$[4]['#name'], 'two' | |||
There was a problem hiding this comment.
Ugh. I must have fat-fingered that line when I was copy-pasting it into a new test. I cut my finger earlier this week, so I'm going to blame the annoying bandage on my finger.
My apologies.
There was a problem hiding this comment.
no worries, can always pass in a quick --amend commit
|
Thanks for the PR! Hmm, good point, though it's quite disappointing that event-emitter is blocking. I plan to review it soon. |
| @@ -402,6 +402,23 @@ class exports.Parser extends events.EventEmitter | |||
| else if @saxParser.ended | |||
| throw err | |||
|
|
|||
| parseStringSync: (str) => | |||
There was a problem hiding this comment.
Can you make the API adhere to the one of the regular parseString and also add the shorthand alias exports.parseStringSync?
No worries, looks mostly alright to me 😃 |
|
Is there anything in particular that's preventing this from being accepted? I'd be happy to help where it's needed - I'd really like this functionality for a project that I'm working on. |
|
@mrparkers None of my comments were addressed, so that's the most blocking issue at the moment. |
|
Okay. I've never used CoffeeScript before but I can try to take a stab at your comments. |
|
Overall I do plan to migrate away from CoffeeScript but while at it I'd like to do some serious re-engineering so that might take quite a while still. |
Create a sync version of parseString to API to permit
calling without a callback.
The current implemenation is sync due to underlying
implementation of SAX parser.
Examples:
```js
// Via root API
var result = xml2js.parseStringSync('< ... >', options)
// Via parser
var parser = new xml2js.Parser(options);
var result = parser.parseStringSync('< ... >');
```
See Leonidas-from-XIV#241 by @nobodyname
See Leonidas-from-XIV#319 by @mrparkers
Create a sync version of parseString to API to permit
calling without a callback.
The current implemenation is sync due to underlying
implementation of SAX parser.
Examples:
```js
// Via root API
var result = xml2js.parseStringSync('< ... >', options)
// Via parser
var parser = new xml2js.Parser(options);
var result = parser.parseStringSync('< ... >');
```
See Leonidas-from-XIV#241 by @nobodyman
See Leonidas-from-XIV#319 by @mrparkers
Create a sync version of parseString to API to permit
calling without a callback.
The current implemenation is sync due to underlying
implementation of SAX parser.
Examples:
```js
// Via root API
var result = xml2js.parseStringSync('< ... >', options)
// Via parser
var parser = new xml2js.Parser(options);
var result = parser.parseStringSync('< ... >');
```
See Leonidas-from-XIV#241 by @nobodyman
See Leonidas-from-XIV#319 by @mrparkers
(note: the history of this PR goes into more detail here: #159 (comment))
Though the async parseString() method is likely the preferred way to parse an xml file, I had need for an async version so that I could use it inside a karma.config.js file (which cannot use async calls).
My understanding is that sax.js uses event-emitter, which is a blocking api. If that's the case(and let me know if I'm wrong), I believe that it's safe to assume that sax events fire events synchronously when you call write() with a string argument instead of a buffer.
Based on this understanding I wrote parseStringSync() which is little more than a wrapper around parseString. Instead of utilizing a callback it accepts a string argument and returns the parsed result. If the underlying call results in an error, parseStringSync throws that error. I also wrote a couple of (simple) tests to assert the sync behavior works as I hoped.
Unfortunately it is my first attempt at writing coffeescript, so I'm sure there's a more elegant way to pull this off than my patch. Feel free to hack it to pieces or reject it outright :-)