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

Added MacOS support #16

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

Conversation

biggoldendragon
Copy link

Added Readme to explain how to use this package on MacOS. Updated exportNotebook to support MacOS. I can't test on Linux to ensure the changes are nondestructive.

@florian-wagner
Copy link
Contributor

Hi @biggoldendragon,

thanks for your contribution. I don't have the possibility to test this on a Mac, but the changes look reasonable. Just one suggestion. Wouldn't it make more sense to use ls -lah "${filename}.pdf" at the end. This would automatically give the file size in a human-readable format and is not restricted to K.

@biggoldendragon
Copy link
Author

Hi @florian-wagner, I agree with your change for computing the filesize variable. I've made the change, tested that it works great on OS X and check-in the updated code on this branch. Thanks!

@florian-wagner
Copy link
Contributor

Hi @biggoldendragon,

thanks for your changes. I tested your Pull request and I get

Error: no such file "Cannot find ./rM2svg"

Why did you introduce the ./? That means that the script needs to be called from its folder. Its much more convenient to set the PATH variable to the scripts and then call it from wherever one wants to export a notebook.

@biggoldendragon
Copy link
Author

Hi @florian-wagner,

I introduced ./ because the exportNotebook script in the main branch produces the error Cannot find rM2svg when I run it on OS X. Are you saying that for the exportNotebook script to run, the maxio/tools directory needs to be in the shell's PATH? If so, we should add that to the instructions.

Also, on the main branch, line 22 says print "Cannot find rM2svg" but that should be echo "Cannot find rM2svg" since the script is running bash instead of python. Of course that bug will only become visible if the code runs down that branch... which it does on OS X but perhaps not on Linux.

My branch changed that section of the script, but perhaps I've changed way to much in an effort to get the script running?

@mseri
Copy link

mseri commented Jun 14, 2018

I suggest an alternative way to support osx here: https://github.com/reHackable/maxio/pull/22/files

@@ -62,7 +69,7 @@ fi
sed -i -e "s/^[[:blank:]]*$/Blank/" "${tmpfolder}"/*.pagedata

filename=$(grep -F '"visibleName"' "${tmpfolder}"/*.metadata | cut -d: -f2- | grep -o '"[^"]*"')
echo "Exporting notebook ${filename} ($(wc -l "${tmpfolder}"/*.pagedata | cut -d\ -f1) pages)"
echo "Exporting notebook ${filename} ($(ls -1 "${tmpfolder}"/*.pagedata | wc -l | awk '{print $1}') pages)"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ls does -1 automatically in a pipe so it's not needed here

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.

4 participants