From 76196dbb01040d31ea96fabadde09ab7d7d9a85a Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Thu, 3 Oct 2024 22:16:04 +0200
Subject: [PATCH] opts: parseKeyValueFile: cleanup and remove redundant
 trimming

- the function already trimmed leading whitespace from each line before
  parsing. keys with trailing whitespace would be invalidated, and values
  have whitespace preserved, so there's no need to trim whitespace for the
  key.
- if a line is validated (key is valid), we don't need to reconstruct the
  key=value by concatenating, and we can add the line as-is.
- check if the key is empty before checking if it contains whitespace
- touch-up comments
- rename some variables for readability
- slight cleanup to use early returns / early continues to reduce nesting

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 opts/file.go | 63 +++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/opts/file.go b/opts/file.go
index d1089ea41d..535839aa7a 100644
--- a/opts/file.go
+++ b/opts/file.go
@@ -12,7 +12,7 @@ import (
 
 const whiteSpaces = " \t"
 
-func parseKeyValueFile(filename string, emptyFn func(string) (string, bool)) ([]string, error) {
+func parseKeyValueFile(filename string, lookupFn func(string) (string, bool)) ([]string, error) {
 	fh, err := os.Open(filename)
 	if err != nil {
 		return []string{}, err
@@ -21,45 +21,48 @@ func parseKeyValueFile(filename string, emptyFn func(string) (string, bool)) ([]
 
 	lines := []string{}
 	scanner := bufio.NewScanner(fh)
-	currentLine := 0
 	utf8bom := []byte{0xEF, 0xBB, 0xBF}
-	for scanner.Scan() {
+	for currentLine := 1; scanner.Scan(); currentLine++ {
 		scannedBytes := scanner.Bytes()
 		if !utf8.Valid(scannedBytes) {
-			return []string{}, fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", filename, currentLine+1, scannedBytes)
+			return []string{}, fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", filename, currentLine, scannedBytes)
 		}
 		// We trim UTF8 BOM
-		if currentLine == 0 {
+		if currentLine == 1 {
 			scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom)
 		}
-		// trim the line from all leading whitespace first
+		// trim the line from all leading whitespace first. trailing whitespace
+		// is part of the value, and is kept unmodified.
 		line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace)
-		currentLine++
-		// line is not empty, and not starting with '#'
-		if len(line) > 0 && !strings.HasPrefix(line, "#") {
-			variable, value, hasValue := strings.Cut(line, "=")
 
-			// trim the front of a variable, but nothing else
-			variable = strings.TrimLeft(variable, whiteSpaces)
-			if strings.ContainsAny(variable, whiteSpaces) {
-				return []string{}, fmt.Errorf("variable '%s' contains whitespaces", variable)
-			}
-			if len(variable) == 0 {
-				return []string{}, fmt.Errorf("no variable name on line '%s'", line)
-			}
+		if len(line) == 0 || line[0] == '#' {
+			// skip empty lines and comments (lines starting with '#')
+			continue
+		}
 
-			if hasValue {
-				// pass the value through, no trimming
-				lines = append(lines, variable+"="+value)
-			} else {
-				var present bool
-				if emptyFn != nil {
-					value, present = emptyFn(line)
-				}
-				if present {
-					// if only a pass-through variable is given, clean it up.
-					lines = append(lines, strings.TrimSpace(variable)+"="+value)
-				}
+		key, _, hasValue := strings.Cut(line, "=")
+		if len(key) == 0 {
+			return []string{}, fmt.Errorf("no variable name on line '%s'", line)
+		}
+
+		// leading whitespace was already removed from the line, but
+		// variables are not allowed to contain whitespace or have
+		// trailing whitespace.
+		if strings.ContainsAny(key, whiteSpaces) {
+			return []string{}, fmt.Errorf("variable '%s' contains whitespaces", key)
+		}
+
+		if hasValue {
+			// key/value pair is valid and has a value; add the line as-is.
+			lines = append(lines, line)
+			continue
+		}
+
+		if lookupFn != nil {
+			// No value given; try to look up the value. The value may be
+			// empty but if no value is found, the key is omitted.
+			if value, found := lookupFn(line); found {
+				lines = append(lines, key+"="+value)
 			}
 		}
 	}