search

ststeiger / PdfSharpCore

Port of the PdfSharp library to .NET Core - largely removed GDI+ (only missing GetFontData - which can be replaced with freetype2)
Other
1.06k stars 235 forks source link

Implement missing PdfDocument.Flatten() method #322

Closed KaasKop97 closed 1 year ago

KaasKop97 commented 1 year ago

Was refactoring a .NET framework project to .NET core and stumbled upon this missing method.

Ported it over from PdfSharp.

packdat commented 1 year ago

This only sets the AcroFields to read-only. (and only on the outermost hierarchy-level, child-fields are not handled) "Real" flattening is a completely different animal.

See this post regarding flattening in the PdfSharp-Forum: https://forum.pdfsharp.net/viewtopic.php?f=3&t=3778#p11641

If you'd like to include this change, please consider renaming the method. Something like MakeAcroFieldsReadOnly or something that better describes what the method actually does.

KaasKop97 commented 1 year ago

Whilst I agree with your point, the orginal PdfSharp uses the same 'Flatten' name.

Nevertheless I'd be more than happy to change the name to the one you propositioned, but I am unsure how this project looks at breaking compatibility between the original and this version?

packdat commented 1 year ago

I'm aware of the fact that the code was ported from the original PdfSharp. However IMHO the original implementation is flawed and the name is misleading. Because of that i would vote against porting this in it's current state.

What if someone comes up with a more correct implementation of flattening in the future ? Does he need to call this new method ReallyFlatten ?

Regarding compatibility: Please correct me, but i would be surprised, if this library is 100% API-compatible with the original one. One method more or less shouldn't be a big problem, especially since I assume this particular method is not used very often.

ststeiger commented 1 year ago

@packdat: I agree with KaasKop97. Use the other name. Don't call it flatten when it doesn't flatten.

ststeiger commented 1 year ago

Merged.