[lldb-dev] command script import
jingham at apple.com
Fri Oct 14 13:25:17 PDT 2011
Sorry, a few more quibbles.
Why not add FileSpec::GetFilenameStrippingExtension or something like that as well, since that's what you really use? That seems like another useful function. Don't take out the extension one, that also seems good.
What a pain that you can't import a Python module w/o adding its containing directory to the Python Path! There must be some reason why, but sigh...
Why do you have to do this bit:
+ FILE *tmp_fh = (python_interpreter->m_dbg_stdout ? python_interpreter->m_dbg_stdout : stdout);
+ Locker py_lock(python_interpreter, tmp_fh);
That seems non-obvious and at least merits a comment.
Finally since there seem to be several reasons why LoadScriptingModule could fail in this function, it would be better if it took an Error & parameter and filled that out appropriately.
On Oct 14, 2011, at 12:55 PM, Enrico Granata wrote:
> Hi all,
> hopefully the third time is the charm :)
>> The only quibble I have is that you do a bunch of path manipulation by hand here that should be done through FileSpec.
>> Also, instead of pulling off the .pyc or .py extension by hand it would be good to add this functionality to FileSpec, since that is also generally useful.
> I have modified the implementation following Jim's advice, and now I use FileSpec directly. I have also added a method named GetFilenameExtension() to said class.
>> As to the test case, no, it does not belong in the python_api subdirectory. You can create a
>> functionalities/command_script subdir to house the import dir.
> It looks like you had already created functionalities/command_script. I have created an import subdirectory there and moved my test case files there.
> Thanks for the feedback on this patch.
> - Enrico Granata
More information about the lldb-dev