Skip to content

Introspection support#30

Merged
death merged 12 commits intodeath:masterfrom
kkatsuyuki:introspection
Nov 4, 2023
Merged

Introspection support#30
death merged 12 commits intodeath:masterfrom
kkatsuyuki:introspection

Conversation

@kkatsuyuki
Copy link
Contributor

I implemented introspection using mixins and added an example file for how to use it. I saw
https://gist.github.com/death/1c5780c5dcf6a838a867d8701969982f
#28

, and I tried to meet what was discussed.
Let me know any comments/requests for introspection addition. I'll consider adjusting my code.

@death
Copy link
Owner

death commented Oct 17, 2023

Hi kkatsuyuki, thank you for the pull request!

For now, I've only skimmed it but will look into it more thoroughly in the future, hopefully next week. It looks good, but two small remarks are (i) :import-from clauses should be added to the defpackage form in publish.lisp for cxml and uiop and any other third-party libraries as needed and (ii) any tab characters should be replaced with spaces.

@kkatsuyuki
Copy link
Contributor Author

I made (i) and (ii) changes.

@death
Copy link
Owner

death commented Oct 23, 2023

Thanks kkatsuyuki.

I looked at the code again, and I have some thoughts:

Maybe it would be a good idea for dbus-object to inherit from these mixins by default, so that they become an implementation detail (and so their names need not be exported, and the publish-introspection example could be fused into the publish example). The specification already mentions that DBUS objects form a hierarchy. I don't see why Introspectable should not be implemented by default; if desired there could be a way to override this decision. Do you agree?

Before merging this change I would like to have a different implementation for relative-path-string. The current implementation conflates DBUS object paths with Common Lisp pathnames/namestrings which are underspecified and not necessarily compatible with DBUS object path syntax. An implementation that performs the necessary string processing without reference to the latter concepts would make sense to me.

The current implementation does not support DBUS object redefinition very well. This will not prevent me from merging this change, but users should know this and are encouraged to submit patches.

@kkatsuyuki
Copy link
Contributor Author

Maybe it would be a good idea for dbus-object to inherit from these mixins by default, so that they become an implementation detail (and so their names need not be exported, and the publish-introspection example could be fused into the publish example). The specification already mentions that DBUS objects form a hierarchy. I don't see why Introspectable should not be implemented by default; if desired there could be a way to override this decision. Do you agree?

Though I don't have so much experiences of using dbus, I don't come up with the story that introspectable becomes harmful, so I agree. I'll adjust the code.

Before merging this change I would like to have a different implementation for relative-path-string. The current implementation conflates DBUS object paths with Common Lisp pathnames/namestrings which are underspecified and not necessarily compatible with DBUS object path syntax. An implementation that performs the necessary string processing without reference to the latter concepts would make sense to me.

I also wondered if the implementation was good, so I understand your comment. I'll modify it.

@kkatsuyuki
Copy link
Contributor Author

I've figured out that I simplified the code too much, so let me modify a little bit more.

@death
Copy link
Owner

death commented Nov 4, 2023

Thank you kkatsuyuki.

@death death merged commit 8bba6a0 into death:master Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants