Skip to content

Conversation

@jcfr
Copy link
Member

@jcfr jcfr commented Sep 7, 2018

This commit ensures an exception is raised when the python script does not
exist. When associated with the --exit-after-startup option, a non-zero status
code is now returned when Slicer exit.

This commit ensures an exception is raised when the python script does not
exist. When associated with the --exit-after-startup option, a non-zero status
code is now returned when Slicer exit.
@jcfr
Copy link
Member Author

jcfr commented Sep 7, 2018

Cc: @lassoan @ihnorton

Copy link
Contributor

@lassoan lassoan left a comment

Choose a reason for hiding this comment

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

Would it be possible to throw the error from inside this->corePythonManager()->executeFile()?

Execution can go wrong in many ways and it is odd that one particular error is reported with an external mechanism.

At least we should let executeFile decide if there is an error (file does not exist, not readable, etc) instead of just checking outside if the file exists. This would be also more robust, when script is defined by relative path (executeFile might be able to find it, while QFile::exists cannot).

@jcfr
Copy link
Member Author

jcfr commented Sep 8, 2018

Thanks for the review

Would it be possible to throw the error from inside this->corePythonManager()->executeFile()

Definitively, that makes sense.

For reference:

https://github.com/commontk/CTK/blob/6c0eddaaf8b306c5cf553e8421bca7a5640e380a/Libs/Scripting/Python/Core/ctkAbstractPythonManager.cpp#L303-L333

@jcfr jcfr added the wip label Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants