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

Resolves #240 #260

Merged
merged 1 commit into from
Aug 19, 2020
Merged

Resolves #240 #260

merged 1 commit into from
Aug 19, 2020

Conversation

angsch
Copy link
Member

@angsch angsch commented Aug 13, 2020

Add two more export options, 'export selection to individual files' requested in #240 and 'export all pages to individual files'.

@Nokia808
Copy link

@angsch
Thank you for this PR !
Only one note: with "export to individual files" options, the output files should be without the outlines (TOC) that may some time already existing in original (input) PDF. For example, if I have a PDF has outlines (TOC) for it's 17 chapters & their sub-chapters, it will be meaningless if each of extracted individual files (that each of them represent a single page of original PDF) has TOC of original file.

I'm nearly sure you are already addressed this, but since I'm not programmer I give this note to avoid post implementation fix ....

Best regards.

@kbengs
Copy link
Member

kbengs commented Aug 14, 2020

Some observations when I tested it:

  • When starting this is printed to terminal: "SyntaxWarning: "is" with a literal. Did you mean "=="? if exportmode is 'ALL_TO_SINGLE':"
  • Mnemonics accelerators are missing in menu.ui (Keyboard shortcuts, Alt + X) Could be for example "Export Specia_l…"
  • "export selection" and "save as" are always greyed-out in hamburger menu (or am I missing something?)
  • Export options are only available in right-click menu, shouldn't they be in hamburger menu also?

But overall I think it looks good.

@angsch
Copy link
Member Author

angsch commented Aug 15, 2020

@kbengs
Thanks for testing. It is really good that you point of the thing with the hamburger menu. I am gonna fix your points. Thanks, again.

@jeromerobert
Copy link
Member

Thank you @angsch. You PR looks good. Here is what I would add to kbengs comments:

  • I think "Export selection" is already something "special". So I would prefer that you rename "Export Special..." to "Export..." and move "Export Selection" to it.
  • I also think that the new export submenu should be in the hamburger menu.
  • Could you also rebase your branch to master and remove the merge commit ?

 * Extend export modes to a total of 4. 1) standard save mode, 2) export
   all pages to individual files, 3) selection to a single PDF, 4) selection
   to individual files
 * Add 'Export special' option to user inteface, offering
   'export all pages to individual pages' and
   'export selection to individual pages'
@angsch
Copy link
Member Author

angsch commented Aug 18, 2020

The code has been revised according to your comments. I think it is ready for being merged.

@kbengs
Copy link
Member

kbengs commented Aug 18, 2020

Two small things I still notice:

  • You probably forgot to remove the line: "attribute name="target" type="i">0</attribute" from the "save as" section in menu.ui
    because save as in hamburger menu is still always greyed-out
  • Empty line in menu.ui line 320

@kbengs
Copy link
Member

kbengs commented Aug 19, 2020

CTRL+F is usually shortcut for find, maybe something else should be used here?

@jeromerobert
Copy link
Member

I merge it, we'll fix that kind of details.

@jeromerobert jeromerobert merged commit 53448e9 into pdfarranger:master Aug 19, 2020
@jeromerobert
Copy link
Member

56b3150

@jeromerobert jeromerobert added this to the 1.7.0 milestone Aug 19, 2020
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.

None yet

4 participants