Skip to content

Add pdf_combine.R file with pdf_combine function. #53

Merged
pridiltal merged 5 commits into
pridiltal:masterfrom
prdm0:pdf_combine
Sep 16, 2021
Merged

Add pdf_combine.R file with pdf_combine function. #53
pridiltal merged 5 commits into
pridiltal:masterfrom
prdm0:pdf_combine

Conversation

@prdm0
Copy link
Copy Markdown
Contributor

@prdm0 prdm0 commented May 6, 2021

The pdf_combine function has good characteristics for combining multiple PDF files. We often want to combine these files by delimiting the beginning and end of each file, that is, combining sequences of pages that often have different sizes in each file. The proposed pdf_combine function does this in a simple and useful way, simply passing multiple paths to the files to the vec_input argument. The page delimitations are passed to the start_pages and end_pages arguments, respectively.

There is a generic example in the documentation of the proposed function.

prdm0 added 3 commits May 6, 2021 13:12
@prdm0
Copy link
Copy Markdown
Contributor Author

prdm0 commented Aug 17, 2021

Hello, any positions regarding this pull request?

@pridiltal
Copy link
Copy Markdown
Owner

Thank you @prdm0 for this pull request!. This proposed pdf_combine function is kind of an extension of the staplr::staple_pdf function. The main difference between the two functions is that pdf_combine allows us to pass each file's start and end pages, which is not possible with the existing staple_pdf function. This will be a really good addition to the package.

Is there any possibility for us to find another name for the proposed function highlighting its unique features as staple_pdf and combine_pdf sound similar. For naming the functions we use the format action_object (eg: staple_pdf, rotate_pdf, rename_files)

@prdm0
Copy link
Copy Markdown
Contributor Author

prdm0 commented Aug 18, 2021

Thank you @prdm0 for this pull request!. This proposed pdf_combine function is kind of an extension of the staplr::staple_pdf function. The main difference between the two functions is that pdf_combine allows us to pass each file's start and end pages, which is not possible with the existing staple_pdf function. This will be a really good addition to the package.

Is there any possibility for us to find another name for the proposed function highlighting its unique features as staple_pdf and combine_pdf sound similar. For naming the functions we use the format action_object (eg: staple_pdf, rotate_pdf, rename_files)

Hi @pridiltal , thanks. I made the change as suggested. I thought it prudent to version the package to 3.2.0 since it's about adding a new feature.

Best regards.

@prdm0
Copy link
Copy Markdown
Contributor Author

prdm0 commented Sep 13, 2021

Dear @pridiltal how are you? What did you think of the changes? Best regards.

@pridiltal
Copy link
Copy Markdown
Owner

Thank you @prdm0 for this new addition. I'll update the package and get back to you soon.

@pridiltal pridiltal merged commit 28ea7ae into pridiltal:master Sep 16, 2021
@pridiltal
Copy link
Copy Markdown
Owner

pridiltal commented Sep 16, 2021

Thank you @prdm0 for this new addition. This will definitely enhance its capabilities. I have a few suggestions to maintain the consistency across functions.

  1. Instead of vec_input, how about using input_directory and input_files as in staple_pdf function. It allows the user to select the input files interactively. This will be very useful if they have a large number of files. Please refer to the staple_pdf function documentation.
  2. Instead of output, can we use output_filepath for the path of the output PDF file. (As in fileIO.R). it allows the user to select the folder interactively.
  3. Sometimes the user might get confused with the two functions staple_pdf and the combine_pdf. We need to explain the purpose of having two functionalities. staple_pdf combines pdf files. combine_pdf allows us to combine pages. It gives some additional capabilities. Instead of calling this new function combine_pdf, how about naming it as staple_pages.
  4. Then we have staple_pdf (old function) to combine PDF FILES. And this new function staple_pages (currently available as combine_pdf) to combine "ONLY the SELECTED PAGES of given input files"

Please let me know your thoughts on this. I am very happy to discuss it further. Once again thank you for joining our team.

Dilini

@prdm0
Copy link
Copy Markdown
Contributor Author

prdm0 commented Sep 16, 2021

Dear @pridiltal, I found the suggestions interesting. As soon as I proceed with the changes, I'll let you know. Thanks you.

@prdm0
Copy link
Copy Markdown
Contributor Author

prdm0 commented Sep 22, 2021

Dear @pridiltal , I made the changes and you are on a new pull request; see #54 .

Best regards.

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

Successfully merging this pull request may close these issues.

2 participants