Adds a new loader for pdf preview #52
Conversation
PdfPreviewLoader is an example of how, with a loader, it possible to preview virtually any file system type. Extending FileSystemLoader, thanks to Lukas's code, PdfPreviewLoader generate a png image of the first page of the pdf document and updates the infos of the file to be rendered.
|
Thanks .. one thing I realized when I got up this morning is that maybe this code should use composition. Aka instead of extending from FileSystemLoader, it should simply get a |
| if (isset($info['extension']) && strpos(strtolower($info['extension']), 'pdf') !== false ) { | ||
| //If it doesn't exists, extract first page of the PDF to png | ||
| if (!file_exists("$absolutePath.png")) { | ||
| $img = new \Imagick ( $absolutePath.'[0]' ); |
There was a problem hiding this comment.
why are you instantiating Imagick here ? Couldn't you do this with Imagine so that it works for people using a different driver ?
There was a problem hiding this comment.
i don't think this will work with other drivers .. but yeah maybe the instance could be fetched from imagine. @avalanche123
There was a problem hiding this comment.
I can confirm that PDF file extension is supported only from Imagick and Gmagick. So the pdf pages can't be extracted with gd.
But I could try to get the used driver and throw an exception if the driver is GD.
There was a problem hiding this comment.
makes sense .. alternatively if the driver is GD, you could also just check if the extension is available and in that case make a new instance and only throw the exception if the extension is also not available
There was a problem hiding this comment.
Ok, I surrender: how can I retrieve the driver's name without injecting anything into PdfPreviewLoader ? (I'm missing those beautiful global variables and classes from S1.4).
Reading directly the configuration file is, obviously, not an option.
There was a problem hiding this comment.
I think I'm getting my head 'round the problem. I had to make some heavy changes but there's still a problem with stof's suggestion: imagine's implementation of Imagick doesn't support the "pdf opening syntax": $absolutePath.'[0]'.
It rises a 500 Server error: "File /home/luca/....../foobar.pdf[0] doesn't exist".
Imagine try to be as agnostic as possible and thus doesn't support Imagick constructor's parameters.
|
@stof I'm not sure that converting a pdf to an image can be done with GD and thus I didn't use Imagine. But I can dive a little bit more. |
|
@lsmith77 That's just an idea but, I'm thinking that PdfPreviewLoader is not strictly a Loader but, let's call it, a transformer. Then, another solution could be to add one more level to the image processing: Loader->Transformer->Filter |
|
Yeah, thats an option, though I worry that we are making things too complicated, which is why I was pondering if the best approach isn't to just use object composition rather than inheritance: This way we could look into making any loader useable with this PDF previewing. |
|
I'm starting to find a profound lack of generality in this pull request. I think this code could find a better position inside an example folder. |
|
it should definitely be possible to do this sort of thing in a clean reusable way. i will try to work on a solution over the course of this week. i guess for now you have at least a solution you can use. |
|
Look at this: 47c825566aaf1aa4d4eff4f0db4c6cee6c6f960d |
|
why statically ? |
|
I refuse to answer and plead the 5th amendment! |
|
static calls are generally harder to test, and makes it a pain if your transformer needs some dependencies. I think using an object is better as some transformers may need to use an object instead of a simple static method |
|
You're absolutely right! And in this way, not only the transformer can be managed by the service container, but can also be an array of transformers! Here's the update: 9b19cf4617c588e96d6cecf9cd9ac573c4c9c354 |
|
I had to cleanup my fork... I pushed a new PR: #57 |
PdfPreviewLoader is an example of how, with a loader, it is
possible to preview virtually any file system type.
Extending
FileSystemLoader, thanks to Lukas's code, PdfPreviewLoadergenerate a png image of the first page of the pdf document and
updates the infos of the file to be rendered.
I think that it would be better to pass from config the number of the page to be extracted and the image format to use but... I'm not sure how this could be done...