[herd][preview] Fix preview multiple files#1909
Conversation
relokin
left a comment
relokin
left a comment
There was a problem hiding this comment.
This is a very useful PR, thanks Hadrien. I've got one suggestion but feel free to merge this as-is.
| | Evince -> do_show_file name_dot "evince" "pdf" | ||
| | Preview -> | ||
| do_show_file ~keep_tmp_pdf:true name_dot "open -a Preview" "pdf" | ||
| run_cmd "%s -Tpdf %s -O && open -a Preview %s.*pdf" generator name_dot |
There was a problem hiding this comment.
Is it worth trying to combine the pdfs into one? Something along the lines of:
| run_cmd "%s -Tpdf %s -O && open -a Preview %s.*pdf" generator name_dot | |
| let join_app = "/System/Library/Automator/Combine\ PDF\ Pages.action/Contents/MacOS/join" in | |
| run_cmd "%s -Tpdf %s -O" generator name_dot ; | |
| run_cmd "%s -o %s %s.*.pdf" join_app name_dot name_dot ; | |
| run_cmd "open -a Preview %s.pdf" name_dot |
There was a problem hiding this comment.
I tried @relokin's nice idea with the following, slightly modified, code.
let join_app =
{| /System/Library/Automator/Combine\ PDF\ Pages.action/Contents/MacOS/join |} in
run_cmd "%s -Tpdf %s -O && %s -o %s.all.pdf %s*.pdf && open -a Preview %s.all.pdf"
generator name_dot
join_app name_dot name_dot
name_dotI like having one window with all diagrams right from the start better then having several windows initially, even if one can regroup them with "Merge all windows" (Menu Window).
There was a problem hiding this comment.
I've implemented something similar in d1319fd. Please have a look and let me know what you think
maranget
left a comment
maranget
left a comment
There was a problem hiding this comment.
LGTM. Consider our comments on merging the PDF files though.
HadrienRenaud
force-pushed
the
fix-preview-multiple-files
branch
from
056a1b4 to
87ebf0c
Compare
maranget
left a comment
maranget
left a comment
There was a problem hiding this comment.
Worked for me and code looks fine.
…ecutions are shown Fix herd#1907
The previous commits created a pdf file per execution shown, and the previous commit merged them into a single PDF file. This commit cleans up the files that are merged. Notice that after this commit, there is no difference when showing a single pdf file.
HadrienRenaud
force-pushed
the
fix-preview-multiple-files
branch
from
9cea122 to
8a83b1c
Compare
3 participants
Also:
-debug prettyAfter implementing the changes suggested in review: