Skip to content

Conversation

@coderabbitai
Copy link
Contributor

@coderabbitai coderabbitai bot commented Nov 18, 2025

Docstrings generation was requested by @leaanthony.

The following files were modified:

  • v3/examples/raw-message/main.go
  • v3/pkg/application/application_darwin.go
  • v3/pkg/application/linux_cgo.go
These file types are not supported
  • v3/pkg/application/webview_window_darwin.m
ℹ️ Note

CodeRabbit cannot perform edits on its own pull requests yet.

Docstrings generation was requested by @leaanthony.

* #4710 (comment)

The following files were modified:

* `v3/examples/raw-message/main.go`
* `v3/pkg/application/application_darwin.go`
* `v3/pkg/application/linux_cgo.go`
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Nov 18, 2025

Deploying wails with  Cloudflare Pages  Cloudflare Pages

Latest commit: de31f81
Status: ✅  Deploy successful!
Preview URL: https://8eb0db49.wails.pages.dev
Branch Preview URL: https://coderabbitai-docstrings-c366.wails.pages.dev

View logs

@coderabbitai
Copy link
Contributor Author

coderabbitai bot commented Nov 18, 2025

Important

Review skipped

CodeRabbit bot authored PR detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

⚠️ Missing Changelog Update

Hi @coderabbitai[bot], please update v3/UNRELEASED_CHANGELOG.md with a description of your changes.

This helps us keep track of changes for the next release.

@coderabbitai
Copy link
Contributor Author

coderabbitai bot commented Nov 18, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the changelog file exists and read its contents to understand the format
if [ -f "v3/UNRELEASED_CHANGELOG.md" ]; then
    echo "=== Changelog file exists ==="
    cat v3/UNRELEASED_CHANGELOG.md
else
    echo "=== Changelog file does not exist ==="
fi

Length of output: 1441


@github-actions, thank you for the reminder! However, as noted in the PR description, I cannot perform edits on my own pull requests yet.

Here's the suggested changelog entry that can be added manually to v3/UNRELEASED_CHANGELOG.md under the Changed section:

## Changed
- Add comprehensive docstrings to raw message handler origin verification functions across Darwin, Linux, and example code (#4717)

This entry should be placed under the "Changed" section since it improves documentation for existing functionality related to origin verification in the raw message handler.

Could a maintainer please add this entry to the changelog, or let me know if you'd prefer different wording?


🧠 Learnings used
Learnt from: APshenkin
Repo: wailsapp/wails PR: 4480
File: v2/internal/frontend/desktop/darwin/message.h:17-19
Timestamp: 2025-08-08T09:13:16.916Z
Learning: In Wails v2 bindings origin verification, processBindingMessage intentionally has different signatures across platforms: Darwin includes an isMainFrame bool (WKWebKit provides it), Linux uses two params (message, source) as WebKitGTK doesn’t expose main-frame info there, and Windows handles origin checks in Go via WebView2 sender/args without a C bridge. This divergence is acceptable/expected per maintainer (APshenkin).

@sonarqubecloud
Copy link

func processMessage(windowID C.uint, message *C.char) {
// processMessage enqueues a message from a webview into the internal window message buffer,
// including the message origin and whether it came from the main frame.
// If `origin` is nil, Origin in the queued OriginInfo is set to the empty string.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeRabbit you can't remove //export processMessage or any //export line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leaanthony You're absolutely right, and I apologize for that error. The //export directive is a critical CGo compiler directive that makes Go functions callable from C code. Without it, the C bridge would break.

The //export directives must be preserved. Documentation comments should be added before the //export line, like this:

// processMessage enqueues a message from a webview into the internal window message buffer,
// including the message origin and whether it came from the main frame.
// If `origin` is nil, Origin in the queued OriginInfo is set to the empty string.
//export processMessage
func processMessage(windowID C.uint, message *C.char, origin *C.char, isMainFrame bool) {
    // ... function body
}

I'll remember this for all CGo code reviews going forward.


✏️ Learnings added
Learnt from: leaanthony
Repo: wailsapp/wails PR: 4717
File: v3/pkg/application/application_darwin.go:349-349
Timestamp: 2025-11-21T07:21:52.534Z
Learning: In Go CGo code, the `//export FunctionName` directive must never be removed from functions that are called from C code. This directive is a compiler instruction that makes the Go function callable from C. Documentation comments should be placed before the `//export` line, not replace it.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: APshenkin
Repo: wailsapp/wails PR: 4480
File: v2/internal/frontend/desktop/darwin/message.h:17-19
Timestamp: 2025-08-08T09:13:16.916Z
Learning: In Wails v2 bindings origin verification, processBindingMessage intentionally has different signatures across platforms: Darwin includes an isMainFrame bool (WKWebKit provides it), Linux uses two params (message, source) as WebKitGTK doesn’t expose main-frame info there, and Windows handles origin checks in Go via WebView2 sender/args without a C bridge. This divergence is acceptable/expected per maintainer (APshenkin).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants