-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat(tools): evaluate\selector\screenshot #10
Conversation
zeoxisca
commented
Apr 2, 2025
- beautify tool key, use global const key to constraint tool name
- add createTab\evaluate\selector\screenshot handler functions
LGTM |
tools/common.go
Outdated
) | ||
) | ||
|
||
type ToolHandler = func(rodCtx *types.Context) func(context.Context, mcp.CallToolRequest) (*mcp.CallToolResult, error) | ||
|
||
var ( | ||
CreateTabHandler = func(rodCtx *types.Context) func(context.Context, mcp.CallToolRequest) (*mcp.CallToolResult, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CreateTableHandler
seems to duplicate the Navigation function. So far, do you want to open multiple pages in a browser at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multiple pages must be taken into account
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multiple pages must be taken into account
Of course, it in the plan. however, there are some problems with your implementation: we should think that mange the all pages, not replace old page with new page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest splitting Pr first and submitting it in batches. 'beautify tool key', this item make a single pr.
I suggest turning off this PR. You can add a new PR except for |
Done, createTab removed |
types/context.go
Outdated
@@ -74,6 +75,7 @@ type Context struct { | |||
config Config | |||
browser *rod.Browser | |||
page *rod.Page | |||
element *rod.Element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you add this what do you want to do? I didn't see where it were used
tools/common.go
Outdated
if element == nil || element.Object == nil { | ||
return nil, errors.New(fmt.Sprintf("Failed to find element %s: %s", selector, err.Error())) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here has some problems?
tools/common.go
Outdated
if element == nil || element.Object == nil { | ||
return nil, errors.New(fmt.Sprintf("Failed to find element: %s", selector)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same as fill handler