search

monperrus / ExpandAnimations

LibreOffice/OpenOffice.org extension to expand animations before exporting to PDF. Looking for maintainers.
https://github.com/monperrus/ExpandAnimations
GNU Lesser General Public License v3.0
230 stars 30 forks source link

fetch TEMP environment variable #30

Closed rhaschke closed 4 years ago

rhaschke commented 5 years ago

As pointed out here, the hardcoded /tmp folder only works on unix-like systems. This PR evaluates the environment variable TEMP, which should be set on Windows to a reasonable value by default. Hopefully fixes #29. @katz-hm, could you, please, give it a try?

rhaschke commented 5 years ago

Done. Could you give it another try?

katz-hm commented 5 years ago

Almost correct. I didnt realize it before, but there is one slash missing. It should be

newUrlExpanded = "file:///" + sTemp + "/" + sDocFileNameWithoutExtension + "-expanded.odp"

(note that this would require to remove the trailing slash from "/tmp" in the Linux case).

Thanks for doing this. Much appreciated.

rhaschke commented 5 years ago

Done. No need to drop the leading slash for Linux.

katz-hm commented 5 years ago

Works on Windows 7. Great, thank you. Green light from me.

Just out of curiosity: If you don't remove the slash, don't you get "file:////tmp/..." ? Is that a valid RFC 8089 URI?

rhaschke commented 5 years ago

If you don't remove the slash, don't you get "file:////tmp/..." ? Is that a valid RFC 8089 URI?

It's not a problem to have a path like //foo///bar, at least on Linux. I think the same holds for windows.

katz-hm commented 5 years ago

You're right, I looked it up. POSIX defines multiple slashes in local paths to be treated as one.

monperrus commented 5 years ago

thanks a lot! LGTM. Are you both OK to merge?

katz-hm commented 5 years ago

absolutely.

rhaschke commented 5 years ago

Yes, this should be ready for merging.

monperrus commented 4 years ago

Thanks a lot. Pushed 0.6 in f52e88f