<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Hi all,</div><div><br><blockquote type="cite"><div>Sorry, a few more quibbles. <br><br>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.<br></div></blockquote><div><br></div><div>I added that in the attached patch file.</div><br><blockquote type="cite"><div><br>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...<br><br></div></blockquote><div><br></div><div>I guess Python is trying to be smart in handling dependencies (that way, if you import foo, and foo needs to import bar, as long as bar is in the path you need not worry about its location on disk). Unfortunately, it looks like there is no trivial way to override that.</div><br><blockquote type="cite"><div>Why do you have to do this bit:<br><br>+ FILE *tmp_fh = (python_interpreter->m_dbg_stdout ? python_interpreter->m_dbg_stdout : stdout);<br>+ <br>+ {<br>+ Locker py_lock(python_interpreter, tmp_fh);<br><br>That seems non-obvious and at least merits a comment.<br><br></div></blockquote><div><br></div><div>This is part of the original implementation of the interaction between LLDB and Python. Basically, Python is single-threaded and LLDB tries to acquire the lock before sending commands. I am not sure about the reason why a file handle to which to write some text is required (informative for the user? trying to enforce a wait that the optimizer will not strip?), but this behavior is consistently replicated throughout ScriptInterpreterPython, so I guess the original coder of that class had her motives.</div><div>A good point would be to move the code that calculates tmp_fh into the Locker class itself. IMHO that would be best served by writing a separate patch because there are so many methods doing exactly that code snippet (I can deal with that once we're done working on this patch if you want).</div><br><blockquote type="cite"><div>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.<br><br></div></blockquote><div><br></div><div>There it is.</div><div>I have also taken the time to check against double import. If we try to double import a same module, Python will report no error, and yet the second import silently does nothing. Now I do check for that condition, and the new command *does* report an error on double import (this would not be harmful in itself, but because we would end up calling __lldb_module_init twice, there might be unwelcome side effects in doing that).</div><br><blockquote type="cite"><div>Jim<br><br><br></div></blockquote><br></div><div>Thanks,</div><div><div><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Euclid; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div>- <i>Enrico Granata</i></div></span></div><br><div></div></div></body></html>