Summary: | [PATCH] The current picture handling uses raw integers for types and index, replace with enum and reference | ||
---|---|---|---|
Product: | POI | Reporter: | mark.o <Mark.Olesen> |
Component: | XSLF | Assignee: | POI Developers List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | enhancement | Keywords: | PatchAvailable |
Priority: | P2 | ||
Version: | 3.13-dev | ||
Target Milestone: | --- | ||
Hardware: | PC | ||
OS: | All | ||
Bug Depends on: | |||
Bug Blocks: | 59836 | ||
Attachments: |
patch as basic coding example
Patch for X/HSLF which uses the (H/XSLF)PictureData element addPicture() with File and InputStream parameters |
Description
mark.o
2015-07-29 11:42:32 UTC
Created attachment 32942 [details]
patch as basic coding example
Created attachment 32952 [details]
Patch for X/HSLF which uses the (H/XSLF)PictureData element
Thank you for bringing this up. I've changed a few things - I hope the new handling is ok for you. Applied with r1693825 No problems with the handling - the main thing is that the integers index is gone, everything else should be encapsulated enough that nobody is bothered. BTW: the PictureType enum has an "ooxml-id". I don't know that this is necessary at all. I believe it was only the integer value into the previous RELATIONS[] lookup, but doesn't have any intrinsic meaning ... it certainly isn't used anywhere in the code. (In reply to Andreas Beeker from comment #3) > Thank you for bringing this up. > I've changed a few things - I hope the new handling is ok for you. > Applied with r1693825 I've reopened with a patch for improved consistency/convenience. I assume that I'll like many users and my image source is file-based. In this case it makes sense to support this directly (as is already done in HSLFSlideShow). I personally find this is much clearer. Created attachment 32954 [details]
addPicture() with File and InputStream parameters
Applied via r1745073, thanks for the work on the patches. It would be good if you can add some unit-tests which cover these new methods so we can close this bug for good! (In reply to Dominik Stadler from comment #7) > Applied via r1745073, thanks for the work on the patches. It would be good > if you can add some unit-tests which cover these new methods so we can close > this bug for good! Added some unit tests for SlideShow#findPicture and SlideShow.addPicture in r1753065 and r1753073. Is there anything else that needs a unit test in order to close this bug? |