Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get console messages to go code #77

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

stroiman
Copy link

@stroiman stroiman commented Dec 5, 2024

This adds the ability to get console messages to Go, but to do that you need to tap into the "inspector", which is also AFAIK the entry point to controlling the debugger. I just need the console messages. Although it seems that I can also use it to control time; which would be useful for my purpose.

This is still WIP, functionally close to done (for my needs), but I'd appreciate feedback, and opinion on code structure.

I also intend to add code-doc before merging.

Inheriting from InspectorClient

In C++, you must create a class inheriting from InspectorClient. This must then be installed using Inspector::Create, and you need to manually attach to each v8 context.

The InspectorClient has all virtual functions, and the client code overrides the ones they care about.

My idea was to create a Go interface for each capability. The user can choose to implement the functions they care about.

So far, I only care about console messages, so that's what I have now. It it still lacking stack traces, but should be simple enough.

When registering an inspector, you need a name, and a contextGroupId. I have no idea what they are used for, except contextGroupId must be <> 0, otherwise nothing happens. Right now they are hardcoded in C++ code.

That also means that I don't know if it makes sense to control from Go code or not.

The client itself does not seem to be related to an isolate. I don't know if it's valid to use the same client for multiple isolates. The code seems to indicate it's possible.

Utility types

The code uses a StringView type for strings. In reality; they're always utf16le (i.e. no byte order mark). Nothing is documented, so all is from experimenting, and only on one platform, my M2 mac. Btw, there must be a better way of converting []byte to []uint16, but I didn't find anything. Btw, I will add a test case that uses some weird unicode characters that take up more than one element in utf16. I did test that manually, and it did work, but better have it in the tests.

I created a more generic type for registering "callbacks". Just like the Isolate registers function callbacks, which I think should just be replaced with that type; which should be moved somewhere else.

Package name

If we follow the idea to create a separate interface for each capability that the inspector supports, it will be quite a lot of types, not necessarily with a sensible name of their own without some context. Prefixing them with InspectorClient makes them even longer, they're long enough IMHO.

I was wondering if it would make sense to put it in a sub-package, v8go/inspector. The C++ code does have it's own namespace, v8_inspector.

p.s. it's only now I see that I have reformatted function_template.go. I have just opened and saved it

@stroiman stroiman marked this pull request as draft December 5, 2024 09:34
@stroiman
Copy link
Author

stroiman commented Dec 5, 2024

Btw, the initial code here predates the previous PR. I noticed I have created InspectorPtr and InspectorClientPtr types, which really isn't necessary, and I'm in favour of just replacing with *v8Inspector / *v8InspectorClient.

If you expressed your opinion on that topic, I have missed that.

@stroiman
Copy link
Author

stroiman commented Dec 5, 2024

I was just browsing the Go documentation, and I notice that CGo has a mechanism for passing Go values to C, and get it back again:

https://pkg.go.dev/runtime/cgo#Handle

So the registry thing seems unnecessary.

@tommie
Copy link
Owner

tommie commented Dec 7, 2024

Btw, the initial code here predates the previous PR. I noticed I have created InspectorPtr and InspectorClientPtr types, which really isn't necessary, and I'm in favour of just replacing with *v8Inspector / *v8InspectorClient.

If you expressed your opinion on that topic, I have missed that.

No, I didn't. I don't see the point of the Ptr types, but I also don't have a strong opinion.

@stroiman
Copy link
Author

stroiman commented Dec 7, 2024

Allright, I'll replace it with normal pointers, IMHO it's a code convention that adds more code in type declarations with no apparent benefit. I can easily imagine it's a result of finally finding a solution that compiles after struggling with the Go-compatible C-types vs. C++ types problem.

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