https://github.com/medismailben requested changes to this pull request.

Hey @rchamala, thanks for coming up with this! This looks super useful!

I left a few comments with the 3 main things that need to change:

- [ ] Register the scripted sim locator with proper command / API with a 
specific target.
- [ ] Let's not duplicate the `ScriptedPythonInterface::Dispatch` 
implementation in the `ScriptedSymbolLocatorPythonInterface`.
- [ ] Documentation and tutorials ^^
  - [ ] In order to make this discoverable and accessible, we usually provide a 
base class in the lldb python module that the user can derive their class from, 
where are every method has a DocStrings, none-abstract methods have a default 
implementation (so the user don't have to implement every method if that's not 
necessary). You can find some examples in 
[lldb/examples/python/templates](https://github.com/llvm/llvm-project/tree/main/lldb/examples/python/templates),
 then you just have to add the python base class in 
[lldb/bindings/python/CMakeLists.txt](https://github.com/llvm/llvm-project/blob/main/lldb/bindings/python/CMakeLists.txt).
 
  - [ ] Register your scripting python interface as an extension so it shows up 
when running `scripting extension list`. You find take a look at the 
ScriptedProcessPythonInterface 
[source](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.cpp#L216-L234)
 and 
[header](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.h#L75-L77)
  - [ ] Finally, more as a nice to have, write a short tutorial on how to use 
this affordance. You can add a markdown in 
[lldb/docs/use/tutorials](https://github.com/llvm/llvm-project/tree/main/lldb/docs/use/tutorials)
 and after your patch gets merged, it will show up in the 
[website](https://lldb.llvm.org/use/python-reference.html).

https://github.com/llvm/llvm-project/pull/181334
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to