Document tricky design decisions

Add explanatory comments around startup timing, single-instance focus handoff, config migration, and Windows/Linux autostart choices.

The new comments capture why these implementations were chosen, what alternatives were intentionally avoided, and which user-facing problems those tradeoffs solve.
This commit is contained in:
mixeme
2026-06-17 23:01:05 +03:00
parent 2f7bbe4fca
commit eb6a1907e6
4 changed files with 37 additions and 0 deletions
+8
View File
@@ -19,6 +19,11 @@ func SetAutostart(enabled bool, executablePath string, iconPath string) error {
if err != nil { if err != nil {
return err return err
} }
// A desktop scheduler with a tray icon belongs to the graphical session, so
// Linux autostart is implemented through XDG Autostart instead of a systemd
// user service. systemd is tempting because it is explicit and scriptable,
// but it is the wrong owner for a windowed app that should inherit the
// desktop session environment and appear in the tray predictably.
if err := cleanupLegacySystemdAutostart(); err != nil { if err := cleanupLegacySystemdAutostart(); err != nil {
return err return err
} }
@@ -138,6 +143,9 @@ func cleanupLegacyDesktopAutostart() error {
if err != nil { if err != nil {
return err return err
} }
// The old PySentry desktop file is removed proactively instead of tolerated
// alongside the new one. Leaving both files in place would risk duplicate
// launches or confusing status diagnostics after the rename.
if err := os.Remove(desktopPath); err != nil && !os.IsNotExist(err) { if err := os.Remove(desktopPath); err != nil && !os.IsNotExist(err) {
return err return err
} }
+9
View File
@@ -13,6 +13,11 @@ const legacyAutostartName = "PySentry"
const startupShortcutFile = autostartName + ".lnk" const startupShortcutFile = autostartName + ".lnk"
func SetAutostart(enabled bool, executablePath string, iconPath string) error { func SetAutostart(enabled bool, executablePath string, iconPath string) error {
// Windows autostart used to write HKCU\Run values, but that approach became
// brittle once paths with spaces and the "--start-in-tray" argument entered
// the picture. A Startup-folder shortcut stores target path and arguments as
// separate structured fields, so it avoids quoting bugs and more closely
// matches how a user would configure a GUI app by hand.
if err := cleanupLegacyRegistryAutostart(); err != nil { if err := cleanupLegacyRegistryAutostart(); err != nil {
return err return err
} }
@@ -87,6 +92,10 @@ func createStartupShortcut(shortcutPath string, executablePath string, iconPath
if iconPath == "" { if iconPath == "" {
iconPath = executablePath iconPath = executablePath
} }
// WScript.Shell is used here deliberately instead of a third-party Go COM
// wrapper. The PowerShell bridge is not glamorous, but it is already present
// on supported Windows systems and keeps the dependency surface much smaller
// for a project that otherwise aims to stay light.
script := `$shell = New-Object -ComObject WScript.Shell; $shortcut = $shell.CreateShortcut($env:GOSENTRY_SHORTCUT_PATH); $shortcut.TargetPath = $env:GOSENTRY_TARGET_PATH; $shortcut.Arguments = $env:GOSENTRY_ARGUMENTS; $shortcut.WorkingDirectory = $env:GOSENTRY_WORKING_DIRECTORY; $shortcut.IconLocation = $env:GOSENTRY_ICON_PATH; $shortcut.Save()` script := `$shell = New-Object -ComObject WScript.Shell; $shortcut = $shell.CreateShortcut($env:GOSENTRY_SHORTCUT_PATH); $shortcut.TargetPath = $env:GOSENTRY_TARGET_PATH; $shortcut.Arguments = $env:GOSENTRY_ARGUMENTS; $shortcut.WorkingDirectory = $env:GOSENTRY_WORKING_DIRECTORY; $shortcut.IconLocation = $env:GOSENTRY_ICON_PATH; $shortcut.Save()`
command := exec.Command("powershell.exe", "-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", script) command := exec.Command("powershell.exe", "-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", "-Command", script)
command.Env = append(os.Environ(), command.Env = append(os.Environ(),
+5
View File
@@ -81,6 +81,11 @@ func loadOrCreateConfig(paths Paths) (Config, error) {
if _, err := os.Stat(configPath); errors.Is(err, os.ErrNotExist) { if _, err := os.Stat(configPath); errors.Is(err, os.ErrNotExist) {
legacyPath := filepath.Join(paths.AppDir, LegacyConfigFileName) legacyPath := filepath.Join(paths.AppDir, LegacyConfigFileName)
if _, legacyErr := os.Stat(legacyPath); legacyErr == nil { if _, legacyErr := os.Stat(legacyPath); legacyErr == nil {
// The rename from PySentry to GoSentry changed the preferred config
// filename. Read the old file once if it is still present so portable
// installs continue to start without a manual migration step. The
// caller later saves the loaded config back through SaveConfig, which
// naturally rewrites it under gosentry.yaml.
configPath = legacyPath configPath = legacyPath
} else { } else {
return config, writeYAML(paths.ConfigPath, config) return config, writeYAML(paths.ConfigPath, config)
+15
View File
@@ -64,10 +64,17 @@ func Run(startInTray bool) {
w.SetContent(content) w.SetContent(content)
serveSingleInstance(instanceListener, w) serveSingleInstance(instanceListener, w)
if startInTray { if startInTray {
// Autostart launches intentionally stay hidden, so "window shown" would be
// a misleading metric. Record a separate startup event for the tray path
// instead of forcing one timing definition onto two different UX flows.
recordStartup(time.Since(started), false) recordStartup(time.Since(started), false)
a.Run() a.Run()
return return
} }
// Show the window before recording startup time. Measuring earlier, during
// widget construction, looked cheaper in History than the user-perceived
// startup really was. The current point is less abstract: it ends when the
// window has actually been handed to the desktop for display.
w.Show() w.Show()
recordStartup(time.Since(started), true) recordStartup(time.Since(started), true)
a.Run() a.Run()
@@ -112,6 +119,10 @@ func acquireSingleInstance(showExisting bool) (net.Listener, bool) {
connection, dialErr := net.DialTimeout("tcp", singleInstanceAddress, time.Second) connection, dialErr := net.DialTimeout("tcp", singleInstanceAddress, time.Second)
if dialErr == nil { if dialErr == nil {
// The first instance listens only on localhost and understands one tiny
// command: "show". That keeps the implementation dependency-free and easy
// to inspect, which matters more here than introducing a named-pipe or
// platform-specific IPC abstraction just to focus an existing window.
if showExisting { if showExisting {
_, _ = io.WriteString(connection, singleInstanceShowCommand) _, _ = io.WriteString(connection, singleInstanceShowCommand)
} }
@@ -183,6 +194,10 @@ func newMainView(w fyne.Window) (fyne.CanvasObject, func(time.Duration, bool)) {
commandOutputScroll.SetMinSize(fyne.NewSize(520, 160)) commandOutputScroll.SetMinSize(fyne.NewSize(520, 160))
history := newHistoryView(&events) history := newHistoryView(&events)
recordStartup := func(duration time.Duration, windowShown bool) { recordStartup := func(duration time.Duration, windowShown bool) {
// Startup is recorded as an in-memory History event instead of being
// persisted into jobs.yaml. It is session diagnostics, not durable job
// state, and keeping it ephemeral avoids polluting the human-editable YAML
// file with process-lifetime bookkeeping.
detail := "Window shown in " + duration.Round(time.Millisecond).String() detail := "Window shown in " + duration.Round(time.Millisecond).String()
if !windowShown { if !windowShown {
detail = "Started in tray in " + duration.Round(time.Millisecond).String() detail = "Started in tray in " + duration.Round(time.Millisecond).String()