Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add async XLinq document/element loading and saving#2436

Merged
stephentoub merged 4 commits intodotnet:futurefrom
stephentoub:new_async_xml
Aug 10, 2015
Merged

Add async XLinq document/element loading and saving#2436
stephentoub merged 4 commits intodotnet:futurefrom
stephentoub:new_async_xml

Conversation

@stephentoub
Copy link
Copy Markdown
Member

This PR replaces #110. I rebased on future, fixed up Cory's changes (a bunch of formatting changes and renames had been merged in the meantime), and then added a few commits of my own, fixing a bug, adding some tests, etc. The tests roundtrip XML by loading and saving via both the sync and async APIs, and then compare the output, using a variety of settings.

cc: @scalablecory, @krwq, @weshaggard, @terrajobst

scalablecory and others added 4 commits July 21, 2015 10:06
Adds XElement.Load/SaveAsync and XDocument.Load/SaveAsync. Code from the sync versions has been largely lifted out so they can share an implementation as much as possible.
The primary set of fixes here is making the new async methods that do argument validation actually do the validation in a non-async method so that argument exceptions propagate synchronously rather than being captured into the returned tasks.  In some cases, the rest of the method is pushed off into a private helper that the public method then calls; in other cases the "async" wasn't strictly necessary and can be removed to help with performance.
It wasn't overriding WriteToAsync, and as such was picking up the base XText's WriteToAsync behavior, which resulted in incorrect output for CDATA.
@stephentoub stephentoub added api-needs-work API needs work before it is approved, it is NOT ready for implementation 2 - In Progress labels Jul 21, 2015
@stephentoub
Copy link
Copy Markdown
Member Author

@krwq, can you please review?

@krwq
Copy link
Copy Markdown
Member

krwq commented Aug 10, 2015

Looks good to me! I didn't see anything obviously wrong and the tests seemed good. Thanks Steve!

@stephentoub
Copy link
Copy Markdown
Member Author

Thanks, Krzysztof!

stephentoub added a commit that referenced this pull request Aug 10, 2015
Add async XLinq document/element loading and saving
@stephentoub stephentoub merged commit 8d55bda into dotnet:future Aug 10, 2015
@stephentoub stephentoub deleted the new_async_xml branch August 10, 2015 23:18
@tmds
Copy link
Copy Markdown
Member

tmds commented Aug 12, 2015

This is a really nice feature. Will it be part of a beta release in the near future?

@tmds
Copy link
Copy Markdown
Member

tmds commented Aug 14, 2015

@stephentoub, see my question above, any idea which milestone will have this feature?

@stephentoub
Copy link
Copy Markdown
Member Author

I don't know. @weshaggard, can you comment? Thanks.

@weshaggard
Copy link
Copy Markdown
Member

@tmds at this point it isn't clear when this will be available in a release. We aren't in a good place yet with release packages from the open so this change would need to be adapted and brought internally by the area owner, which is @krwq in this case, because we would need to rationalize how we handle this with the full desktop framework which doesn't contain these changes, yet.

stephentoub added a commit to stephentoub/corefx that referenced this pull request Aug 18, 2015
stephentoub added a commit to stephentoub/corefx that referenced this pull request Aug 18, 2015
@tmds
Copy link
Copy Markdown
Member

tmds commented Sep 1, 2015

@weshaggard has there been some progress? Would it be possible to set a milestone?

btw, CoreCLR and full framework co-existance and release cycles seem like an interesting topic for a blog post!

@tmds
Copy link
Copy Markdown
Member

tmds commented Sep 15, 2015

@weshaggard is there some progress?
Is there a way for me to build this so I can use it on 4.5.1? Or isn't that an option because this package comes with the full framework?

@weshaggard
Copy link
Copy Markdown
Member

Sorry for the delay. Unfortunately there isn't any real progress to report on this at this time. There is also no way for you to consume these changes on .NET 4.5.1, given that these APIs ship in the box there and it does not contain them. You will only be able to consume them on a version of the full .NET framework once they are added directly there. That takes some time unfortunately. If you are interested in progress about getting these APIs merged please follow #2869, but I will warn you that it will take a while as we have other higher priority items we are currently working on.

@tmds
Copy link
Copy Markdown
Member

tmds commented Sep 16, 2015

@weshaggard Thanks for the info. If I understand correctly, using it on the full desktop framework means waiting for the next release of that framework. What about the coreclr, can this (and the other features of #2869) be included in the beta builds/releases?

@weshaggard
Copy link
Copy Markdown
Member

You can of course build them yourself and run them on CoreCLR/.NET Core but we don't have all plans to push these into our official package yet. There is still a fair amount of coupling between the full framework and .NET Core that we are trying to untangle which is why we are still not building our packages externally on GitHub yet. It is a goal of mine to be able to produce our packages on GitHub by the end of the year at which time we can potentially be able to include these in the our official packages.

@ajbeaven
Copy link
Copy Markdown

How much further along is this one now? Any chance of an update?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api-needs-work API needs work before it is approved, it is NOT ready for implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants