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

Export enums for type/command/event properties/params #16

Open
mafredri opened this issue Jul 13, 2017 · 1 comment
Open

Export enums for type/command/event properties/params #16

mafredri opened this issue Jul 13, 2017 · 1 comment

Comments

@mafredri
Copy link
Owner

mafredri commented Jul 13, 2017

Some types, commands and events contain unnamed enums. Unnamed meaning they do not reference any domain type.

Take debugger.PausedReply for example, it contains a Reason of type string. This is actually an enum without a name defined in the protocol by:

{ "name": "reason", "type": "string", "enum": [ "XHR", "DOM", "EventListener", "exception", "assert", "debugCommand", "promiseRejection", "OOM", "other", "ambiguous" ], "description": "Pause reason." }

This could easily be translated into:

// PausedReason Pause reason.
type PausedReason int

// PausedReason as enums.
const (
	PausedReasonNotSet PausedReason = iota
	PausedReasonXHR
	PausedReasonDOM
	PausedReasonEventListener
	PausedReasonException
	PausedReasonAssert
	PausedReasonDebugCommand
	PausedReasonPromiseRejection
	PausedReasonOOM
	PausedReasonOther
	PausedReasonAmbiguous
)

// ...

One problem here is naming. Since the struct (or event) is called paused, and the parameter is called reason, I think it makes sense to name it PausedReason. This works well for the most part, but can result in some really long names, EmulateTouchFromMouseEventButtonNone, or in the worst case, names that do not work at all: ForcePseudoStateForcedPseudoClassesActive.

If we were to name them differently, e.g. just Reason it would not work with e.g. ObjectPreview.Subtype (name Subtype) since this name is shared between ObjectPreview, PropertyPreview and RemoteObject. Also, the enum for RemoteObject differs from ObjectPreview and PropertyPreview, which seems like a bug.

Since some props/params share enums it might be nice to have these types as part of the domain, and it would also ease with the naming of things.

I think the next step is to report/request this to the chrome-debugging-protocol mailing list.

@mafredri
Copy link
Owner Author

This situation has been slightly improved with e91c341, the documentation now specifies what enum values are possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant