From bd9ff6078651816bd63ae012e60cbe25f3facd1a 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 | 55 ++++++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/opts/file.go b/opts/file.go
index d1089ea41d14..a88f7dca3ed3 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
@@ -32,34 +32,39 @@ func parseKeyValueFile(filename string, emptyFn func(string) (string, bool)) ([]
 		if currentLine == 0 {
 			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 || strings.HasPrefix(line, "#") {
+			// skip empty lines and comments (lines starting with '#')
+			continue
+		}
+
+		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 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)
-				}
+		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)
 			}
 		}
 	}