-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: add dlvflag for golang debug #4410
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughIntroduces a new dlvflag option for dev runs and updates restart logic to optionally launch via Delve. Adds an internal conv package with BytesToString and replaces many string([]byte) conversions across the codebase. Fixes a byte-to-string conversion in a dev watcher. Updates changelog. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Dev CLI
participant Restart as restartApp
participant OS as OS Process
Dev->>Restart: run (appBinary, args, DlvFlag)
alt DlvFlag empty
Note over Restart: Launch app directly
Restart->>OS: StartProcess(appBinary, args)
else DlvFlag provided
Note over Restart: Launch via Delve
Restart->>Restart: dlvArgs = shlex.Split(DlvFlag)
Restart->>Restart: newArgs = dlvArgs + ["exec", appBinary] + ["--", args...]
Restart->>OS: StartProcess("dlv", newArgs)
end
OS-->>Restart: pid / error
Restart-->>Dev: result (with existing cleanup/error handling)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
v2/cmd/wails/flags/dev.go (1)
34-34
: Consider enhancing the flag description for better user experience.The flag addition is implemented correctly and follows the established pattern. However, the description could be more informative about the expected format or provide usage examples.
Consider updating the description to be more descriptive:
- DlvFlag string `flag:"dlvflag" description:"Flags to pass to dlv"` + DlvFlag string `flag:"dlvflag" description:"Flags to pass to dlv (space-separated, e.g., '--listen=:40000 --headless')"`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
v2/cmd/wails/flags/dev.go
(1 hunks)v2/cmd/wails/internal/dev/dev.go
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
v2/cmd/wails/internal/dev/dev.go (1)
v2/internal/process/process.go (1)
NewProcess
(16-24)
v2/cmd/wails/internal/dev/dev.go
Outdated
|
||
var cmd string | ||
if len(f.DlvFlag) == 0 { | ||
cmd = appBinary | ||
} else { | ||
cmd = "dlv" | ||
newArgs := append(strings.Split(f.DlvFlag, " "), "exec", appBinary) | ||
if len(args) > 0 { | ||
newArgs = append(newArgs, "--") | ||
args = append(newArgs, args...) | ||
} | ||
args = newArgs | ||
} | ||
|
||
buildOptions.Logger.Print("NewProcess cmd: %s, args: %v", cmd, args) | ||
newProcess := process.NewProcess(cmd, args...) |
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.
🛠️ Refactor suggestion
Improve argument parsing robustness and add basic validation.
The core logic for dlv integration is correct and follows the proper command structure. However, there are opportunities for improvement:
-
Argument parsing: Using
strings.Split(f.DlvFlag, " ")
doesn't handle quoted arguments properly, which could break with complex flags like--listen=:40000 --api-version=2
. -
Missing validation: No check if
dlv
is available on the system.
Consider this improved implementation:
var cmd string
+ var newArgs []string
if len(f.DlvFlag) == 0 {
cmd = appBinary
} else {
cmd = "dlv"
- newArgs := append(strings.Split(f.DlvFlag, " "), "exec", appBinary)
+ // Use shlex to properly handle quoted arguments
+ dlvArgs, err := shlex.Split(f.DlvFlag)
+ if err != nil {
+ buildOptions.Logger.Fatal("Unable to parse dlvflag: %s", err.Error())
+ }
+ newArgs = append(dlvArgs, "exec", appBinary)
if len(args) > 0 {
newArgs = append(newArgs, "--")
- args = append(newArgs, args...)
+ newArgs = append(newArgs, args...)
}
- args = newArgs
+ args = newArgs
}
This uses the already imported shlex
package for proper argument parsing and fixes the argument construction logic.
🤖 Prompt for AI Agents
In v2/cmd/wails/internal/dev/dev.go around lines 312 to 327, the current code
uses strings.Split to parse f.DlvFlag, which fails to handle quoted arguments
correctly, and it lacks validation to check if the 'dlv' command is available on
the system. To fix this, replace strings.Split with the shlex package to
properly parse f.DlvFlag into arguments respecting quotes, and add a check
before running the command to verify that 'dlv' is installed and accessible,
returning an error or logging appropriately if not found.
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.
@gongzhxu ☝️ This is a good suggestion. Please could you add an entry to the changelog located at website/src/pages/changelog.mdx
?
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Thanks 🙏 Please could you add an entry to the changelog located at |
|
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
v2/cmd/wails/generate.go
(2 hunks)v2/cmd/wails/internal/dev/dev.go
(3 hunks)v2/internal/binding/binding_test/binding_conflicting_package_name_test.go
(2 hunks)v2/internal/binding/binding_test/binding_returned_promises_test.go
(2 hunks)v2/internal/binding/binding_test/binding_type_alias_test.go
(2 hunks)v2/internal/conv/conv.go
(1 hunks)v2/internal/frontend/desktop/darwin/clipboard.go
(2 hunks)v2/internal/frontend/desktop/darwin/frontend.go
(2 hunks)v2/internal/frontend/desktop/linux/frontend.go
(2 hunks)v2/internal/frontend/desktop/linux/single_instance.go
(2 hunks)v2/internal/frontend/desktop/windows/frontend.go
(1 hunks)v2/internal/frontend/desktop/windows/single_instance.go
(2 hunks)v2/internal/frontend/desktop/windows/winc/w32/utils.go
(1 hunks)v2/internal/frontend/dispatcher/calls.go
(3 hunks)v2/internal/frontend/dispatcher/securecalls.go
(2 hunks)v2/internal/menumanager/contextmenu.go
(2 hunks)v2/internal/menumanager/processedMenu.go
(2 hunks)v2/internal/menumanager/traymenu.go
(3 hunks)v2/internal/s/s.go
(2 hunks)v2/pkg/commands/bindings/bindings_test.go
(2 hunks)v2/pkg/commands/build/base.go
(2 hunks)v2/pkg/git/git.go
(2 hunks)v2/tools/release/release.go
(3 hunks)
✅ Files skipped from review due to trivial changes (9)
- v2/pkg/commands/build/base.go
- v2/internal/frontend/desktop/darwin/clipboard.go
- v2/cmd/wails/generate.go
- v2/internal/menumanager/traymenu.go
- v2/internal/menumanager/contextmenu.go
- v2/internal/frontend/dispatcher/calls.go
- v2/tools/release/release.go
- v2/pkg/git/git.go
- v2/internal/frontend/dispatcher/securecalls.go
🚧 Files skipped from review as they are similar to previous changes (1)
- v2/cmd/wails/internal/dev/dev.go
🧰 Additional context used
🧠 Learnings (8)
v2/internal/binding/binding_test/binding_returned_promises_test.go (1)
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/info.go:28-68
Timestamp: 2025-02-04T23:59:43.956Z
Learning: In the Wails v3 project, internal functions are designed to panic on nil parameters as they represent contract violations, rather than adding defensive nil checks.
v2/pkg/commands/bindings/bindings_test.go (1)
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/info.go:28-68
Timestamp: 2025-02-04T23:59:43.956Z
Learning: In the Wails v3 project, internal functions are designed to panic on nil parameters as they represent contract violations, rather than adding defensive nil checks.
v2/internal/menumanager/processedMenu.go (2)
Learnt from: leaanthony
PR: wailsapp/wails#4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
Learnt from: fbbdev
PR: wailsapp/wails#4066
File: v3/pkg/application/messageprocessor_call.go:174-174
Timestamp: 2025-02-13T01:05:02.267Z
Learning: When handling JSON marshaling errors in Wails v3, the error message from json.Marshal provides sufficient debugging context. Logging raw data is unnecessary and could make logs harder to read.
v2/internal/frontend/desktop/windows/single_instance.go (1)
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications_windows.go:91-113
Timestamp: 2025-02-24T06:08:55.645Z
Learning: The JSON quote handling in Windows notifications' activation arguments (v3/pkg/services/notifications/notifications_windows.go) has a known limitation with single-quote collisions that needs to be addressed after initial testing.
v2/internal/frontend/desktop/linux/frontend.go (2)
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications_windows.go:91-113
Timestamp: 2025-02-24T06:08:55.645Z
Learning: The JSON quote handling in Windows notifications' activation arguments (v3/pkg/services/notifications/notifications_windows.go) has a known limitation with single-quote collisions that needs to be addressed after initial testing.
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications_windows.go:113-164
Timestamp: 2025-02-24T06:23:10.967Z
Learning: In Windows Toast notifications (notifications_windows.go), metadata must be appended to each action's Arguments because the toast activated callback only returns the Arguments string of the clicked action. This is achieved by appending the same escaped JSON data (with single quotes) to all actions using the ":::" delimiter.
v2/internal/frontend/desktop/windows/frontend.go (2)
Learnt from: popaprozac
PR: wailsapp/wails#4098
File: v3/pkg/services/notifications/notifications_windows.go:91-113
Timestamp: 2025-02-24T06:08:55.645Z
Learning: The JSON quote handling in Windows notifications' activation arguments (v3/pkg/services/notifications/notifications_windows.go) has a known limitation with single-quote collisions that needs to be addressed after initial testing.
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/info.go:28-68
Timestamp: 2025-02-04T23:59:43.956Z
Learning: In the Wails v3 project, internal functions are designed to panic on nil parameters as they represent contract violations, rather than adding defensive nil checks.
v2/internal/frontend/desktop/darwin/frontend.go (2)
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/info.go:28-68
Timestamp: 2025-02-04T23:59:43.956Z
Learning: In the Wails v3 project, internal functions are designed to panic on nil parameters as they represent contract violations, rather than adding defensive nil checks.
Learnt from: leaanthony
PR: wailsapp/wails#4031
File: v3/pkg/application/menu.go:199-202
Timestamp: 2025-01-24T22:41:18.566Z
Learning: In the Wails menu system (v3/pkg/application/menu.go), shared state between menus is intentionally designed and desirable. Methods like `Append()` and `Prepend()` should maintain shared references to menu items rather than creating deep copies.
v2/internal/s/s.go (1)
Learnt from: fbbdev
PR: wailsapp/wails#4045
File: v3/internal/generator/render/info.go:28-68
Timestamp: 2025-02-04T23:59:43.956Z
Learning: In the Wails v3 project, internal functions are designed to panic on nil parameters as they represent contract violations, rather than adding defensive nil checks.
🧬 Code Graph Analysis (11)
v2/internal/binding/binding_test/binding_conflicting_package_name_test.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/binding/binding_test/binding_returned_promises_test.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/pkg/commands/bindings/bindings_test.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/binding/binding_test/binding_type_alias_test.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/menumanager/processedMenu.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/frontend/desktop/windows/winc/w32/utils.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/frontend/desktop/windows/single_instance.go (2)
v2/internal/frontend/desktop/windows/winc/w32/user32.go (1)
SendMessage
(371-379)v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/frontend/desktop/linux/frontend.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/frontend/desktop/windows/frontend.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/frontend/desktop/darwin/frontend.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
v2/internal/s/s.go (1)
v2/internal/conv/conv.go (1)
BytesToString
(9-11)
🔇 Additional comments (17)
v2/pkg/commands/bindings/bindings_test.go (1)
11-11
: Inconsistent with PR objectives - these changes are unrelated to dlvflag debugging feature.The PR objectives state this is about adding dlvflag for golang debug, but these changes are replacing byte-to-string conversions with an unsafe utility function. This appears to be a completely different set of changes.
Additionally, while the unsafe conversion is safe here since
goMod
is not reused after line 65, using unsafe optimizations in test code provides minimal benefit and adds unnecessary risk.Also applies to: 65-65
Likely an incorrect or invalid review comment.
v2/internal/s/s.go (1)
14-14
: Safe but unrelated to PR objectives.While this unsafe conversion is safe (the byte slice is not reused), these changes are unrelated to the stated PR objective of adding dlvflag for golang debugging.
Also applies to: 267-267
Likely an incorrect or invalid review comment.
v2/internal/binding/binding_test/binding_conflicting_package_name_test.go (1)
13-13
: Unrelated to PR objectives.These changes replace string conversion with unsafe optimization, which is unrelated to the stated PR objective of adding dlvflag for golang debugging.
Also applies to: 61-61
Likely an incorrect or invalid review comment.
v2/internal/binding/binding_test/binding_returned_promises_test.go (1)
9-9
: Unrelated to PR objectives.These changes are part of the byte-to-string conversion optimization pattern, not the dlvflag debugging feature mentioned in the PR objectives.
Also applies to: 78-78
Likely an incorrect or invalid review comment.
v2/internal/conv/conv.go (1)
1-11
: This file is completely unrelated to the PR objectives of adding dlvflag for golang debugging.The PR description mentions adding debugging support with Delve, but this file introduces an unsafe string conversion utility. This appears to be an entirely different feature.
Additionally, while the unsafe conversion can improve performance by avoiding allocation, it comes with significant risks that should be better documented:
Consider adding more comprehensive documentation:
// BytesToString converts []byte to string without memory allocation. // WARNING: The returned string must be used as read-only! +// +// SAFETY: This function is only safe to use when: +// 1. The byte slice will not be modified after conversion +// 2. The byte slice will outlive the returned string +// 3. The caller does not store the string beyond the lifetime of the byte slice +// +// DO NOT use this function with: +// - Byte slices that may be modified +// - Temporary byte slices that may be garbage collected +// - Byte slices from pools that may be reused func BytesToString(b []byte) string {Likely an incorrect or invalid review comment.
v2/internal/binding/binding_test/binding_type_alias_test.go (2)
8-9
: LGTM: Import reorganization follows Go conventions.The external package imports were correctly moved below standard library imports, and the new
conv
package import was added appropriately.
61-61
: LGTM: Consistent byte-to-string conversion optimization.The change from
string(rawGeneratedBindings)
toconv.BytesToString(rawGeneratedBindings)
is correct and aligns with the broader refactoring to use zero-allocation string conversions throughout the codebase.v2/internal/frontend/desktop/windows/single_instance.go (2)
11-14
: LGTM: Import reorganization improves code structure.The external package imports were correctly grouped together at the bottom of the import block, following Go conventions.
67-67
: LGTM: Optimized byte-to-string conversion in IPC context.The change from
string(serialized)
toconv.BytesToString(serialized)
is appropriate for the JSON-marshaled data being passed toSendMessage
. This optimization eliminates unnecessary allocation during inter-process communication.v2/internal/menumanager/processedMenu.go (2)
6-6
: LGTM: Added required import for conversion utility.The
conv
package import was correctly added to support the new byte-to-string conversion function.
138-138
: LGTM: Zero-allocation JSON marshaling optimization.The change from
string(menuAsJSON)
toconv.BytesToString(menuAsJSON)
optimizes the conversion of JSON-marshaled bytes to string without affecting the method's functionality or error handling.v2/internal/frontend/desktop/linux/frontend.go (2)
7-7
: LGTM: Cleaned up cgo directive formatting.Removed trailing space in the cgo directive, improving code cleanliness without affecting functionality.
494-494
: LGTM: Optimized string conversion in JavaScript callback.The change from
string(escaped)
toconv.BytesToString(escaped)
eliminates allocation when converting JSON-escaped message data for JavaScript execution, consistent with the broader optimization effort.v2/internal/frontend/desktop/linux/single_instance.go (2)
12-13
: LGTM: Import reorganization follows Go conventions.The reordering of imports to separate standard library from third-party packages is correct.
72-72
: LGTM: Efficient byte-to-string conversion optimization.The replacement of
string(serialized)
withconv.BytesToString(serialized)
provides zero-allocation conversion for the D-Bus message payload, which is appropriate since the string is used immediately in the API call.v2/internal/frontend/desktop/darwin/frontend.go (2)
29-29
: LGTM: Conv package import added for optimization.The addition of the conv package import supports the byte-to-string conversion optimization.
428-428
: LGTM: Optimized callback message conversion.The replacement of
string(escaped)
withconv.BytesToString(escaped)
provides zero-allocation conversion for the JSON callback payload, improving performance in the JavaScript execution path.
} | ||
f.mainWindow.Invoke(func() { | ||
f.chromium.Eval(`window.wails.Callback(` + string(escaped) + `);`) | ||
f.chromium.Eval(`window.wails.Callback(` + conv.BytesToString(escaped) + `);`) |
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.
💡 Verification agent
🧩 Analysis chain
LGTM: Optimized callback conversion, but verify import.
The replacement of string(escaped)
with conv.BytesToString(escaped)
provides efficient conversion for the JavaScript callback. However, ensure the conv
package import is properly added to this file.
🏁 Script executed:
#!/bin/bash
# Description: Verify conv package import in windows frontend file
# Expected: Should find the conv import declaration
rg -A 5 -B 5 "github.com/wailsapp/wails/v2/internal/conv" v2/internal/frontend/desktop/windows/frontend.go
Length of output: 106
To confirm whether the conv
package is actually imported and how it’s referenced, let’s inspect the import block and any occurrences of conv
in the file:
🏁 Script executed:
#!/bin/bash
# Show the import section (with context)
rg -A20 -B0 "^import" v2/internal/frontend/desktop/windows/frontend.go
# List all lines where conv is referenced
rg -n "conv\." v2/internal/frontend/desktop/windows/frontend.go
Length of output: 677
Missing conv
import in Windows frontend
The call to conv.BytesToString
on line 812 will fail without importing the conv
package. Please add the following import to v2/internal/frontend/desktop/windows/frontend.go
:
import (
// existing imports…
+ "github.com/wailsapp/wails/v2/internal/conv"
)
This ensures conv.BytesToString(escaped)
resolves correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
f.chromium.Eval(`window.wails.Callback(` + conv.BytesToString(escaped) + `);`) | |
import ( | |
// existing imports… | |
+ "github.com/wailsapp/wails/v2/internal/conv" | |
) |
🤖 Prompt for AI Agents
In v2/internal/frontend/desktop/windows/frontend.go at line 812, the code uses
conv.BytesToString but the conv package is not imported. To fix this, add an
import statement for the conv package at the top of the file so that
conv.BytesToString(escaped) resolves correctly and the code compiles without
errors.
@gongzhxu - do you still want to merge this in? There's a few comments to address above 🙏 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v2/cmd/wails/internal/dev/dev.go (1)
504-504
: Avoid unsafe zero-copy here or document the guarantee.Using conv.BytesToString is unsafe unless the backing []byte is guaranteed immutable thereafter. Here it’s likely fine, but the perf gain is negligible and readability/safety suffer.
Apply this safer change:
- assetDir = conv.BytesToString(content) + assetDir = string(content)If you keep the zero-copy, add a brief comment stating the slice will not be mutated after this point.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
v2/cmd/wails/flags/dev.go
(1 hunks)v2/cmd/wails/internal/dev/dev.go
(3 hunks)v2/internal/frontend/desktop/darwin/frontend.go
(2 hunks)v2/internal/frontend/desktop/linux/frontend.go
(1 hunks)v2/internal/frontend/desktop/windows/frontend.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- v2/internal/frontend/desktop/darwin/frontend.go
- v2/internal/frontend/desktop/windows/frontend.go
- v2/cmd/wails/flags/dev.go
- v2/internal/frontend/desktop/linux/frontend.go
🧰 Additional context used
🧬 Code graph analysis (1)
v2/cmd/wails/internal/dev/dev.go (2)
v2/internal/process/process.go (1)
NewProcess
(16-24)v2/internal/conv/conv.go (1)
BytesToString
(9-11)
🔇 Additional comments (2)
v2/cmd/wails/internal/dev/dev.go (2)
26-26
: Import looks fine.The conv package is used below (Line 504).
318-334
: Fix loss of app args and robustly parse dlv flags; also validate dlv availability.
- Bug: Original app args are dropped when dlv is enabled (Line 327 sets args but later overwritten by Line 329).
- Use shlex.Split to correctly handle quoted dlv flags.
- Treat whitespace-only dlv flags as empty.
- Validate that dlv exists in PATH before attempting to run.
Apply this diff:
- var command string - if len(f.DlvFlag) == 0 { - command = appBinary - } else { - command = "dlv" - newArgs := append(strings.Split(f.DlvFlag, " "), "exec", appBinary) - if len(args) > 0 { - newArgs = append(newArgs, "--") - args = append(newArgs, args...) - } - args = newArgs - } + var command string + if strings.TrimSpace(f.DlvFlag) == "" { + command = appBinary + } else { + // Ensure dlv is installed + if _, lookErr := exec.LookPath("dlv"); lookErr != nil { + buildOptions.Logger.Fatal("Delve (dlv) not found in PATH: %s", lookErr.Error()) + } + command = "dlv" + dlvArgs, splitErr := shlex.Split(f.DlvFlag) + if splitErr != nil { + buildOptions.Logger.Fatal("Unable to parse dlvflag: %s", splitErr.Error()) + } + newArgs := append(dlvArgs, "exec", appBinary) + if len(args) > 0 { + newArgs = append(newArgs, "--") + newArgs = append(newArgs, args...) + } + args = newArgs + }Note: Document that users should not include the “exec” subcommand in -dlvflag; it is injected by Wails.
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/src/pages/changelog.mdx
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T13:15:20.339Z
Learnt from: APshenkin
PR: wailsapp/wails#4484
File: v2/internal/frontend/utils/urlValidator.go:25-31
Timestamp: 2025-08-08T13:15:20.339Z
Learning: In Wails v2 (PR wailsapp/wails#4484), for BrowserOpenURL URL validation (v2/internal/frontend/utils/urlValidator.go), maintainers (APshenkin) prefer not to restrict schemes to an http/https allowlist because supported schemes may vary. The allowlist suggestion is declined; continue with the existing denylist approach and generic validation.
Applied to files:
website/src/pages/changelog.mdx
- Add `dlvflag` for golang debug in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu | ||
- Standardized string conversions to avoid subtle memory/runtime in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu |
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.
Clarify the second changelog bullet
The sentence currently stops at “memory/runtime”, so readers don’t know what was avoided. Please finish the thought (e.g. “…issues”).
-- Standardized string conversions to avoid subtle memory/runtime in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu
+- Standardized string conversions to avoid subtle memory/runtime issues in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- Add `dlvflag` for golang debug in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu | |
- Standardized string conversions to avoid subtle memory/runtime in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu | |
- Add `dlvflag` for golang debug in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu | |
- Standardized string conversions to avoid subtle memory/runtime issues in [PR](https://github.com/wailsapp/wails/pull/4410) by @gongzhxu |
🤖 Prompt for AI Agents
In website/src/pages/changelog.mdx around lines 24 to 25, the second changelog
bullet ends with “memory/runtime” and is incomplete; edit the sentence to finish
the thought (for example: “Standardized string conversions to avoid subtle
memory/runtime issues in [PR](https://github.com/wailsapp/wails/pull/4410) by
@gongzhxu”), ensuring the phrasing is clear and grammatically complete.
Thanks to @leaanthony for the reply. I just saw this message and have made the improvements. Please help review it again, and I will make timely adjustments if there are any issues. |
Summary
Resolves
[Closes https://github.com/wailsapp/wails/issues/2740](#3623)
USAGE
wails dev -dlvflag '--listen=:2345 --headless=true --api-version=2 --accept-multiclient'
IDE Goland
add Shell script to startup wails program

add Go Remote

add Compound to run above two

debug the program

TODO
Summary by CodeRabbit