Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

Option to omit page number in output file name #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AckerApple
Copy link

I needed my image output file names to match the pdf file names WITHOUT page numbers.

A test and updated docs have been included. I also already moved the patch version number up.

Thank you kindly.

@dustinc
Copy link
Contributor

dustinc commented Oct 3, 2016

@AckerApple I like the idea but what about PDFs with more than one page? Would each additional page overwrite the previous page's image file?

@AckerApple
Copy link
Author

AckerApple commented Oct 3, 2016

First, the README documentation does not cover multi pages. I have only used this package for single pdf pages using the documented .convertPage() method.

Second, I see this guys package depends on yours and handles multi pdfs and multi pages. My commit would not effect this package: https://www.npmjs.com/package/pdf2images-multiple

I completely understand your thought to think of everything but I feel like you're asking me "what should fs.writeFileSync(path, data) do if an existing file exists"... I wish I had another way to say "I think you are over thinking it" because you're not but I just cant think of better examples to illustrate that omitPageNumOnFileName is a secondary option that when used, implies no page numbers will be used so the last file wrote will be your image by implication.

Do hope I didn't over complicate this with my reply.

@dustinc
Copy link
Contributor

dustinc commented Oct 3, 2016

Right, so if you have multiple pages I think what you're wanting to do would in fact overwrite previous pages. So if you have a 2 page PDF your final image would be of just the 2nd page. Of course, if you know you're only going to be using single page PDFs then, you're right, all is well.

I have no affiliation with this repo other than a fork of my own that I maintain for my own projects because @mooz seems to have lost interest in this one, but I would like to keep mine updated with good PRs.

Definitely was not trying to bash your PR, was just asking out of honest interest.

@AckerApple
Copy link
Author

No bashing taken and I know I am direct and don't consider how harsh my own replies can be so if I came off as trying to swat a fly away, I was not.

This pull request is important to me and I hate to hear someone is no longer maintaining a package. I will give a week or so before making bold moves such as forking and NPM publishing my own... ON THIS SUBJECT, I think it's a better idea if authorization to manage this package at its source be given to trusted parties so that in the worst case of programmer death, someone can carry the torch.

Farewell friends. Lets keep making open source the undeniable way to go!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants