Page MenuHomePhabricator

Fix and simplify lldb.command decorator
ClosedPublic

Authored by kastiglione on Jun 27 2018, 9:53 AM.

Details

Summary

This change fixes one issue with lldb.command, and also reduces the implementation.

The fix: a command function's docstring was not shown when running help <command_name>. This is because the docstring attached the source function is not propagated to the decorated function (f.__call__). By returning the original function, the docstring will be properly displayed by help.

Also with this change, the command name is assumed to be the function's name, but can still be explicitly defined as previously.

Additionally, the implementation was updated to:

  • Remove inner class
  • Remove use of inspect module
  • Remove *args and **kwargs

Diff Detail

Repository
rL LLVM

Event Timeline

kastiglione created this revision.Jun 27 2018, 9:53 AM

Update a comment

granata.enrico added a subscriber: granata.enrico.

Thanks for doing this work.

I stopped working on lldb a couple years ago, so I removed myself as a reviewer. Greg will take good care of you I'm sure.

Thanks @granata.enrico, sorry for the disruption, I noticed previous comments after I had submitted this.

Can we add a test for this to ensure all functionality that we want? I recently made changes to the cmdtemplate.py. Maybe we can integrate some more functionality that auto registers a class somehow?

@clayborg I'll dig into how python-extensions.swig gets tested, and add something accordingly.

I have only briefly looked at cmdtemplate.py. I can look into adding similar registration behavior to it. For what it's worth, I prefer functions to classes for defining commands.

kastiglione planned changes to this revision.Jun 29 2018, 11:50 AM

Add tests and make command_name optional

kastiglione edited the summary of this revision. (Show Details)Jul 1 2018, 12:00 PM
clayborg accepted this revision.Jul 1 2018, 10:12 PM
This revision is now accepted and ready to land.Jul 1 2018, 10:12 PM
This revision was automatically updated to reflect the committed changes.