stellarium icon indicating copy to clipboard operation
stellarium copied to clipboard

Cannot create a subfolder for screenshot from a script

Open janbilek opened this issue 1 year ago • 12 comments

Expected Behaviour

When setting a screenshot path it has to exist core.screenshot("fileName", false, "my/new/folder", true);

Actual Behaviour

Stellarium complaints in a console about missing destination folder Error: cannot create screenshot directory: Could not create directory: INFO Saving screenshot in file: "/home/blech/Pictures/Stellarium/stellarium-002.png"

Steps to reproduce

Use following code in a script: core.screenshot("fileName", false, "my/new/folder", true);

System

  • Stellarium version: 7ebcf893ff5eaa90336e5587ec0df1a0af2a1166
  • Operating system: Linux
  • Graphics Card: Irrelevant
  • Screen type (if applicable): Irrelevant

Logfile

N/A

janbilek avatar Aug 13 '22 07:08 janbilek

I have a fix proposal here already. Please do nothing about this.

janbilek avatar Aug 13 '22 07:08 janbilek

Thanks for adding your first issue to Stellarium. If you have questions, please do not hesitate to contact us.

github-actions[bot] avatar Aug 13 '22 07:08 github-actions[bot]

I think we had discussed this, and decided to not allow creation of new directories for safety reasons. @alex-w ?

gzotti avatar Aug 13 '22 08:08 gzotti

Howdy @gzotti & @alex-w - all good & no worries, it was a dinner time here in Brisi and I had some problems with github authentication, but when you'll see it - I am changing practically nothing, clearly not adding nothing new. Please give me another 2 hrs to put kids in bed.

janbilek avatar Aug 13 '22 10:08 janbilek

Howdy again, I am back but it doesn't allow me to post a fix + script - even using a separate branch. I am sorry, but I have no experience with public repositories like this. Would you please provide me with any guidance? I am going to send a patch through the mailing list meanwhile.

janbilek avatar Aug 13 '22 12:08 janbilek

@janbilek sorry, I didn't check the patch, but... are you fixed error message?

alex-w avatar Aug 19 '22 18:08 alex-w

I think we had discussed this, and decided to not allow creation of new directories for safety reasons. @alex-w ?

Yes, but I didn’t check saving screenshots into subdirectories.

alex-w avatar Aug 26 '22 06:08 alex-w

OK, I've checked the patch and: a) the fix is not completed yet and it potentially dangerous b) we does not allow creation of new directories via scripts

Of course we may implement this feature request, but in this case we should add many checks for various platforms by safety and security reasons.

alex-w avatar Sep 12 '22 17:09 alex-w

We could allow it, but only guarded by a user-defined decision (extra boolean non-property FileMgr::flagAllowScreenshotExtraDir or so) which must by default be off. This flag should not be settable by a script!

gzotti avatar Sep 12 '22 20:09 gzotti

Hello @janbilek!

Thank you for suggesting this feature.

github-actions[bot] avatar Sep 13 '22 08:09 github-actions[bot]

Note that STORING to some existing directory is already possible. See the skybox.ssc for example. Not sure if really creation of arbitrary directories is good.

gzotti avatar Sep 13 '22 11:09 gzotti

Thank you @alex-w & @gzotti for dedicating your time to this. So I hope that it is clear why I needed the creation of arbitrary directories. Honestly I think that all this is more like a one-off use case for our project and you should probably keep that conservative side as it proven doing very well so far.

There is an obvious workaround to have those output folders pre-prepared - which fixes it all.

Of course, I'll be honored to be able to contribute a line or two to your project. So if there is any further work on this needed, I'm happy to follow whatever guidance from your side. On other hand I am also old enough to admit that you guys know the best. :)

janbilek avatar Sep 13 '22 23:09 janbilek