Skip to content
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

adding parseStringSync + simple tests (continued) #319

Closed
wants to merge 5 commits into from

Conversation

mrparkers
Copy link

@mrparkers mrparkers commented Aug 12, 2016

Apologies for getting this in so late, I haven't had the free time to work on this until now.

This PR is to address the comments made by @Leonidas-from-XIV in #241. Please refer to that PR for more context around these changes.

I'm pretty sure I got everything, except I was a bit confused by the comment asking for parseStringSync to adhere to the API of parseString. If that could get cleared up for me, I can take a stab at that as well.

Thanks!

@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage decreased (-0.04%) to 96.951% when pulling 45724fc on Mrparkers:master into 129ebba on Leonidas-from-XIV:master.

@Leonidas-from-XIV
Copy link
Owner

Leonidas-from-XIV commented Aug 20, 2016

@mrparkers About that comment: the parseStringSync function does not accept an (optional) argument map, hence has no way to configure the numerous options that xml2js has. I would wish this to be possibly and certainly all future users of that API as well.

(Also, sorry for taking so long, busy week)

@mrparkers
Copy link
Author

Okay, I will try to address this when I have some free time. Thanks!

mckramer added a commit to mckramer/node-xml2js that referenced this pull request Feb 13, 2018
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
mckramer added a commit to mckramer/node-xml2js that referenced this pull request Feb 13, 2018
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
@mckramer mckramer mentioned this pull request Feb 13, 2018
@mckramer
Copy link
Contributor

Rebased these changes and added support for options in #422

@mrparkers
Copy link
Author

Thanks @mckramer, I'll close this in favor of your PR. I haven't needed this for quite some time now.

@mrparkers mrparkers closed this Feb 13, 2018
mckramer added a commit to mckramer/node-xml2js that referenced this pull request Mar 12, 2018
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants