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

target image file name is fixed to "page-" #13

Closed
vtsoft opened this issue Mar 18, 2019 · 20 comments
Closed

target image file name is fixed to "page-" #13

vtsoft opened this issue Mar 18, 2019 · 20 comments

Comments

@vtsoft
Copy link

vtsoft commented Mar 18, 2019

No description provided.

@vtsoft vtsoft changed the title target image file name was fixed to "page-" target image file name is fixed to "page-" Mar 18, 2019
@vtsoft
Copy link
Author

vtsoft commented Mar 18, 2019

Hi,

Thank you for this great conversion utility.

I just like to ask if it is possible to pass a "target file name for the converted images" maybe in the next version?

example:

 function convert($filename); // without extension

       $image_path = $this->output_path."/".$filename."-%d.".$this->imageExtention;

       $fileArray[] = $filename."-$i.".$this->imageExtention;

Thanks in advance

@imalhasaranga
Copy link
Owner

@mablae @ahfeel @joshbmarshall guys can you please see this something we should implement ?

@mablae
Copy link
Contributor

mablae commented Mar 18, 2019

it would be a nice enhancement.

However it needs to be done in a BC secure way or implies new major Version with this approach. (New param would break BC)

Instead introducing an new parameter on convert method it could be done with a new setter method setFilenamePattern defaulting to current pattern.

It also needs to be checked to contain page parameter and then use sprintf to fill the page number later on. This way we could have 01-page.pdf, too.

Let me know what you think.

@joshbmarshall
Copy link
Contributor

You'd definitely do it in the way you currently set resolution - by adding as a setter method. So the default would be page- and the function would let you override. No BC issues then as @mablae suggested. That should be fairly straightforward to implement and use.

Going on conventions of other software I doubt we need to have the page number configurable in the image file name.

Do you want a pull request?

@imalhasaranga
Copy link
Owner

I think I agree with @mablae too, it is the way to go! @joshbmarshall please go ahead if you have time

@joshbmarshall
Copy link
Contributor

pull request has been submitted #14

@imalhasaranga
Copy link
Owner

Can anyone review @joshbmarshall 's code and merge it and create tag for release version 1.2.2, if all are busy I'll have a look at this tonight and probably merge it.. please make sure Tests are passing

@joshbmarshall
Copy link
Contributor

I just added to the pull request, some PHPDoc and add method chaining to allow end users have cleaner code.

@imalhasaranga
Copy link
Owner

@joshbmarshall thanks for the great work!!!

@joshbmarshall
Copy link
Contributor

@imalhasaranga there is one test not passing on the current master with Ghostscript v9.26 on linux - PDFLibTest::testImageToPdf

@imalhasaranga
Copy link
Owner

hmm! we should find the root case and fix it, I mean tests are the only way we know that nothing has broken.. probably this may be an issue existed from the begining and no one noticed it

@imalhasaranga
Copy link
Owner

sorry release should be v1.2.3

@joshbmarshall
Copy link
Contributor

I have found the issue with the gs on linux, putting in a fix now will add to the pull request

@joshbmarshall
Copy link
Contributor

Done, please check tests pass on windows. They all pass on Alpine Linux with PHP7.3.2

@vtsoft
Copy link
Author

vtsoft commented Mar 19, 2019

Thank you for adding this feature. I really needed this.
If it is okay to ask, when would it be the next release for version 1.2.3 be available?

Thanks again and more power to you guys who made this amazing conversion utility.

@mablae
Copy link
Contributor

mablae commented Mar 19, 2019

Since this is a new feature I would release as v1.3.0

@imalhasaranga
Copy link
Owner

@vtsoft v1.3.0 is released, @mablae agreed!! @joshbmarshall awesome work mate!!! cheers

@vtsoft
Copy link
Author

vtsoft commented Mar 20, 2019

@imalhasaranga, @mablae, @joshbmarshall,

Guys, Thanks for the quick release of the new version. It's really a great help.
So far, I'm starting to test/apply on the ff. platforms:
was able to test it in windows 10 pro, and it works like a charm.
no issue encountered so far.
will test it in CentOs 6.8.
but still looking for the right package and version to use on Ghostscript.

If not too much to ask,
I just like to know the link to which version of Ghostscript did you use in CentOs 6.8 during your testing.

Thanks again and more power to you guys.
I know for sure that this is the right place to ask.

@joshbmarshall
Copy link
Contributor

@vtsoft I have not used CentOS 6 for over 4 years. Even CentOS 7 has too old a version. You just need gs version 9.16 or newer. I tested with v9.26. Maybe you can use the GPL linux binaries from https://ghostscript.com/download/gsdnld.html

@vtsoft
Copy link
Author

vtsoft commented Mar 27, 2019

@joshbmarshall Thank you for your help.
PDFlib using Ghostscript is now running in CentOs server.

and Thanks again all of you guys.

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

No branches or pull requests

4 participants