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

Add a package abstraction for System.IO.Packaging #1295

Merged
merged 6 commits into from
Jan 11, 2023
Merged

Conversation

twsouthwick
Copy link
Member

The Package and PackagePart types from System.IO.Packaging are not built in a way to make composition easy; they do not expose public overridable methods. These abstractions will help with smoothing over some issues found in those types that are difficult to manage.

The Package and PackagePart types from System.IO.Packaging are not built in a way to make composition easy; they do not expose public overridable methods. These abstractions will help with smoothing over some issues found in those types that are difficult to manage.
Copy link
Member

@lindexi lindexi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you

Copy link
Collaborator

@mikeebowen mikeebowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a small question about String.Concat() vs +

@@ -76,7 +79,7 @@ protected XDocument ToFlatOpcDocument(XProcessingInstruction? instruction)
new XElement(
Pkg + "package",
new XAttribute(XNamespace.Xmlns + "pkg", Pkg.ToString()),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to use String.Concat() here instead?

@@ -127,7 +130,7 @@ private static XElement GetBinaryPartContentsAsXml(PackagePart part)
new XElement(Pkg + "binaryData", ToChunkedBase64String(part)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to use String.Concat() here instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably - there are many issues with the FlatOpc implementations regarding performance... this PR was focused on replacing things with the ipackage rather than fixing anything else.

Copy link
Member

@lindexi lindexi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you

@michaelcsikos
Copy link

How do I do this now that Package is Obsolete?

var newId = (uint)(spreadsheetDocument.Package.GetRelationships().Count() + 1);
var rId   = "relId" + newId;

...

spreadsheetDocument.Package.CreateRelationship(uri,
                                               TargetMode.Internal,
                                               "http://schemas.openxmlformats.org/officeDocument/2006/relationships/worksheet",
                                               rId);

It would be nice if the Obsolete message pointed us in the right direction. I'm rolling back to 2.19.0 for now.

@eSPiYa
Copy link

eSPiYa commented Apr 27, 2023

So, how to get parts of the packages now?
Our code is like this:

using(var doc = WordprocessingDocument.Open(filepath, true)){
var fontTable = doc.MainDocumentPart.OpenXmlPackage.Package.GetPart(new Uri(@"/word/fontTable.xml", UriKind.Relative));

// some operations here

doc.Close();
}

@XavierNV
Copy link

Same as @eSPiYa ... I used to get a packagepart for a image (mediaUrl) in a presentation with this code
Dim ioPackage As IO.Packaging.Package = presDocument.Package
Dim pckPart = ioPackage.GetPart(mediaUri)
how can i get it now ?

@twsouthwick
Copy link
Member Author

The obsolete has been obsoleted :) I heard enough feedback of people using this that I've reverted that. However, in v3.0, it's a new type with similar shape that should be a drop in replacement (the System.IO.Packaging types are just not as composable as needed)

@twsouthwick
Copy link
Member Author

For now, we're not planning on releasing a v2.20.1 - this is mostly because we had to move to GH actions during this timeframe and we haven't back ported those changes to that branch. If having a v2.20.1 with the obsolete removed would be helpful for migrating to v3.0, please thumbs up this and we can prioritize it.

@XavierNV
Copy link

XavierNV commented Oct 4, 2023

Thanks a lot for your feedback. We'll wait for v3.0 final version...

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.

6 participants