-
Notifications
You must be signed in to change notification settings - Fork 567
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
Solved svg2pdf conversion error if Inkscape is installed into the default path on a windows machine #1469
Conversation
…uble spaces. Unsure if this change impacts Linux or Mac Systems. But solves my problem on windows
The issues raises by the automated checks are part of the solution: The string isn't the same as expected because I added the double quotes. So failing the checks seems okay. Still in question: how does this change impact other operating systems like Unix. |
hey there, I'm on mac os 11.1 and I am not able to export notebook to pdf (I have installed TeX)
|
Thank you MichaelAdolph, I've had the same problem. Please work on getting this merged devs--for now I just edited the source code in my install. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix -- apologies that not many devs have been active on this project recently
I've fixed this and potentially other shell metacharacter issues with #1512. @MichaelAdolph I'd love for you to test that out on windows! |
* Don't use a shell to call inkscape Python generally recommends against using `shell=True` when calling subprocesses (https://docs.python.org/3/library/subprocess.html#security-considerations). This also causes issues with shell metacharacters (see #1469). I'm also not entirely sure that the shell command *is* fully trustable - I don't know where {to_filename} and {from_filename} are from. Rest of nbconvert also prefers using lists instead of strings to call commands. svg2pdf also uses a command instead of a string now. We leave the old string implementation alone for backwards compatibility, although I'd really prefer to remove it. We don't need the quotes set up in #1469, since using a list automatically deals with that. * Fix call to isinstance
The " " in "Program Files" in the default path for Inkscape on a windows machine makes the conversion fail. I updated the file to enclose the command string into double quotes. This allows the system to run properly on windows.
I'm not able to test on a Mac or Linux/Unix machine, so I'm unsure if my solution will create problems for users on different operating systems.