Sync dev to main by tis24dev · Pull Request #132 · tis24dev/proxsave

tis24dev · 2026-02-03T23:00:55Z

Improve env config parsing and upgrade logic
Report config upgrade warnings
Add env parsing utilities and unify setEnvValue
Handle 'export' prefix with arbitrary whitespace
Add test to preserve inline comments in upgrade
Add tabbed whitespace case to config test
Simplify caseConflicts nil check in upgrade
Add test to preserve block comments on upgrade
Normalize env keys to uppercase and handle export

Refactor config parsing and upgrade flow to better handle comments, casing and multiline values. - setEnvValue: skip commented lines, preserve indentation and inline comments when replacing values. - parseEnvFile/parseEnvValues: ignore comment-only lines inside multi-line block values; return comment text and a caseMap (UPPER->original) for case-insensitive matching; add comment storage to envValue. - splitKeyValueRaw: return parsed inline comment, handle legacy "export " prefixes, and more robustly extract quoted/unquoted values and trailing comments. - computeConfigUpgrade: use case-insensitive lookups via caseMap, track processed user keys, preserve user key casing for extras, count preserved values correctly, and detect no-op upgrades by comparing newContent with originalContent. - renderEnvValue: include preserved inline comments when rendering single-line values. - Added import of utils.IsComment where needed. Also updated internal/orchestrator/.backup.lock (pid/time) as part of the changeset.

Add non-fatal warnings support to the config upgrade flow. Introduce a Warnings []string field on UpgradeResult and propagate warning messages from parseEnvValues through computeConfigUpgrade. Detect and warn about duplicate template keys that differ only by case, case-collision between user keys and template keys, and ignored non-KEY=VALUE lines; preserve behavior for multi-line values. Surface these warnings in the CLI (cmd/proxsave/main.go and cmd/proxsave/upgrade.go) so they are printed during plan/upgrade operations. Update parseEnvValues signature to return case conflict info and warnings, and add tests verifying case-collision and ignored-line warnings.

Introduce utils string helpers (FindInlineCommentIndex, FindClosingQuoteIndex, SetEnvValue) to centralize handling of inline comments, escaped hashes and quoted values. Replace local setEnvValue implementations to call utils.SetEnvValue and update SplitKeyValue/splitKeyValueRaw to use the new helpers for robust parsing. Fix case-insensitive handling of block value keys in config upgrade logic (use upperKey). Add unit tests across packages to cover escaped hashes, preserved comments, quoted values, and SetEnvValue behavior. Minor reformatting/import adjustments in the install wizard to integrate the new utils usage.

Replace manual HasPrefix checks with a Fields-based parse to handle legacy "export KEY=VALUE" lines that use arbitrary whitespace (spaces or tabs). This prevents accidentally stripping keys like "exporter" while still recognizing true "export" prefixes. Added tests to ensure export-with-whitespace is accepted and that keys named "exporter" are not treated as exports.

Add TestUpgradeConfigPreservesInlineComments which verifies UpgradeConfigFile preserves inline comments when adding missing keys. The test seeds a temporary config with inline and quoted '#' comments, runs UpgradeConfigFile, asserts result.Changed is true, and checks the upgraded file still contains the original inline comments for KEY1 and KEY2.

Extend TestPlanUpgradeConfigHandlesExportWhitespace in internal/config/upgrade_test.go to include an export line with tab characters (export\t\tKEY1=custom-tabbed). This ensures the upgrade parsing handles export lines with tabs as well as spaces.

Remove the redundant `caseConflicts == nil` check when warning about keys that differ only by case. Reading a nil map in Go is safe, so using `!caseConflicts[upperKey]` preserves behavior while simplifying the condition and reducing clutter in internal/config/upgrade.go.

Adds TestUpgradeConfigPreservesBlockComments to internal/config/upgrade_test.go. The new test seeds a user config containing a multi-line variable with inline comment lines, runs UpgradeConfigFile, asserts that an upgrade occurs (result.Changed == true) and verifies the block-style comment lines and content are preserved in the upgraded file.

parseEnvFile: normalize parsed environment variable names to uppercase and support lines beginning with "export". Lookups against blockValueKeys and multiValueKeys now use the uppercased key, and stored map keys are uppercased so legacy/lowercase env files are handled consistently. Added tests verifying lowercase keys are normalized (including multi-line block values and multi-value accumulation) and that lines prefixed with various spacing for "export" are parsed correctly.

Copilot

Pull request overview

This PR syncs the dev branch to main, implementing comprehensive improvements to environment config parsing and upgrade logic with enhanced support for legacy config formats.

Changes:

Added robust env config parsing utilities with proper handling of inline comments, quoted values, and escaped characters
Implemented case-insensitive key matching and export prefix handling for backward compatibility
Enhanced config upgrade logic to preserve inline/block comments and report warnings for parsing issues

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/utils/strings.go	Added FindInlineCommentIndex, FindClosingQuoteIndex, and SetEnvValue utility functions for parsing and manipulating env files
pkg/utils/strings_test.go	Added comprehensive tests for new utility functions including edge cases for escaped characters and quoted values
internal/config/upgrade.go	Enhanced upgrade logic with case-insensitive matching, export prefix handling, comment preservation, and warning generation
internal/config/upgrade_test.go	Added tests for export whitespace handling, comment preservation, case collisions, and ignored line warnings
internal/config/config.go	Updated parseEnvFile to normalize keys to uppercase and handle export prefix for backward compatibility
internal/config/migration_test.go	Added tests for lowercase key handling and export line parsing
internal/tui/wizard/install.go	Migrated to use utils.SetEnvValue and fixed indentation inconsistencies
internal/tui/wizard/install_test.go	Added tests for comment preservation in setEnvValue
cmd/proxsave/upgrade.go	Added display of config upgrade warnings
cmd/proxsave/main.go	Added warning display for both plan and upgrade operations
cmd/proxsave/config_helpers.go	Migrated to use utils.SetEnvValue
cmd/proxsave/helpers_test.go	Added tests for comment preservation with quoted values and escaped characters
internal/orchestrator/.backup.lock	Runtime lock file that should not be committed

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-03T23:06:24Z

internal/orchestrator/.backup.lock

+pid=99930
 host=pve
-time=2026-02-03T00:07:31+01:00
+time=2026-02-03T19:42:03+01:00


This file appears to be a runtime lock file that should not be committed to version control. Lock files that contain PIDs and timestamps are typically generated at runtime and should be added to .gitignore instead.

Copilot · 2026-02-03T23:06:25Z

pkg/utils/strings.go

+		if len(parts) >= 1 && strings.TrimSpace(parts[0]) == key {
+			// Found match.
+			leadingLen := len(line) - len(strings.TrimLeft(line, " \t"))
+			leading := ""
+			if leadingLen > 0 {
+				leading = line[:leadingLen]
+			}
+
+			comment := ""
+			commentSpacing := ""
+
+			commentIdx := FindInlineCommentIndex(line)
+			if commentIdx >= 0 {
+				comment = line[commentIdx:]
+
+				beforeComment := line[:commentIdx]
+				trimmedBefore := strings.TrimRight(beforeComment, " \t")
+				if len(trimmedBefore) < len(beforeComment) {
+					commentSpacing = beforeComment[len(trimmedBefore):]
+				}
+			}
+
+			newLine := leading + key + "=" + value
+			if comment != "" {
+				spacing := commentSpacing
+				if spacing == "" {
+					spacing = " "
+				}
+				newLine += spacing + comment
+			}
+			lines[i] = newLine
+			replaced = true


The SetEnvValue function doesn't handle lines with the "export" prefix. When matching keys, it should strip "export" prefix like the splitKeyValueRaw function does in upgrade.go. This could cause SetEnvValue to fail to update lines that start with "export KEY=value".

Suggested change

if len(parts) >= 1 && strings.TrimSpace(parts[0]) == key {

// Found match.

leadingLen := len(line) - len(strings.TrimLeft(line, " \t"))

leading := ""

if leadingLen > 0 {

leading = line[:leadingLen]

}

comment := ""

commentSpacing := ""

commentIdx := FindInlineCommentIndex(line)

if commentIdx >= 0 {

comment = line[commentIdx:]

beforeComment := line[:commentIdx]

trimmedBefore := strings.TrimRight(beforeComment, " \t")

if len(trimmedBefore) < len(beforeComment) {

commentSpacing = beforeComment[len(trimmedBefore):]

}

}

newLine := leading + key + "=" + value

if comment != "" {

spacing := commentSpacing

if spacing == "" {

spacing = " "

}

newLine += spacing + comment

}

lines[i] = newLine

replaced = true

if len(parts) >= 1 {

originalKeyPart := parts[0]

trimmedKey := strings.TrimSpace(originalKeyPart)

hasExport := false

keyPart := trimmedKey

if strings.HasPrefix(trimmedKey, "export ") {

hasExport = true

// Strip 'export' and any surrounding whitespace to get the actual key name.

keyPart = strings.TrimSpace(strings.TrimPrefix(trimmedKey, "export"))

}

if keyPart == key {

// Found match.

leadingLen := len(line) - len(strings.TrimLeft(line, " \t"))

leading := ""

if leadingLen > 0 {

leading = line[:leadingLen]

}

comment := ""

commentSpacing := ""

commentIdx := FindInlineCommentIndex(line)

if commentIdx >= 0 {

comment = line[commentIdx:]

beforeComment := line[:commentIdx]

trimmedBefore := strings.TrimRight(beforeComment, " \t")

if len(trimmedBefore) < len(beforeComment) {

commentSpacing = beforeComment[len(trimmedBefore):]

}

}

exportPrefix := ""

if hasExport {

// Preserve spacing between 'export' and the key as in the original line.

afterExport := originalKeyPart[len("export"):]

afterExportTrimmedLeft := strings.TrimLeft(afterExport, " \t")

spacesLen := len(afterExport) - len(afterExportTrimmedLeft)

spaces := ""

if spacesLen > 0 {

spaces = afterExport[:spacesLen]

} else {

// Ensure at least one space between 'export' and the key.

spaces = " "

}

exportPrefix = "export" + spaces

}

newLine := leading + exportPrefix + key + "=" + value

if comment != "" {

spacing := commentSpacing

if spacing == "" {

spacing = " "

}

newLine += spacing + comment

}

lines[i] = newLine

replaced = true

}

Copilot · 2026-02-03T23:06:25Z

internal/config/upgrade_test.go

+		if err != nil {
+			t.Fatalf("failed to read upgraded config: %v", err)
+		}
+		// strings.ReplaceAll might not be available if "strings" isn't imported, but it is imported in upgrade_test.go


This comment is unnecessary since strings.ReplaceAll has been available since Go 1.12 and the strings package is clearly imported at the top of the file.

Suggested change

// strings.ReplaceAll might not be available if "strings" isn't imported, but it is imported in upgrade_test.go

SetEnvValue now detects an optional leading "export" token when parsing env lines and preserves it when rewriting the line. The function trims and normalizes the export prefix (e.g. "export FOO=...") and only updates the value when the parsed key matches. A unit test was added to verify the export prefix is kept and normalized in the resulting line.

codecov · 2026-02-03T23:19:31Z

Codecov Report

❌ Patch coverage is 77.60417% with 43 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/tui/wizard/install.go	3.03%	32 Missing ⚠️
internal/config/upgrade.go	90.90%	4 Missing and 2 partials ⚠️
pkg/utils/strings.go	93.82%	3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

tis24dev added 9 commits February 3, 2026 21:27

Copilot AI review requested due to automatic review settings February 3, 2026 23:00

Copilot started reviewing on behalf of tis24dev February 3, 2026 23:01 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

tis24dev merged commit a2bd350 into main Feb 3, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Sync dev to main#132

Sync dev to main#132
tis24dev merged 10 commits intomainfrom
dev

tis24dev commented Feb 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 3, 2026

Uh oh!

Copilot AI Feb 3, 2026

Uh oh!

Copilot AI Feb 3, 2026

Uh oh!

Uh oh!

codecov bot commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Comments

Conversation

tis24dev commented Feb 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Feb 3, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant