From 0dc5552c73044d1df3e2b61fbcfce5be9064a661 Mon Sep 17 00:00:00 2001 From: Davide Cappellini Date: Sat, 26 Sep 2026 15:18:04 +0200 Subject: [PATCH] j139 dotenv: strip trailing inline comments and warn on non-token values A `#` preceded by whitespace and outside quotes now ends the value, so `TR_DEBUG_DRAW=0 # hides the grid` resolves to `0` instead of the whole tail. Values that are still not plain tokens (whitespace, `#`, an unclosed quote) get one `[dotenv] WARNING` line naming file, key, raw value and the fact that the reader falls back to its DEFAULT, instead of being applied silently. Guard test 29 -> 47 checks. --- ModularBot_garage/src/env_dotenv.nim | 61 ++++++++++++++++++++++++- common_libs/tests/test_env_dotenv.nim | 66 +++++++++++++++++++++++++++ docs/env_reference.md | 4 +- 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/ModularBot_garage/src/env_dotenv.nim b/ModularBot_garage/src/env_dotenv.nim index c9b3d11..049cbed 100644 --- a/ModularBot_garage/src/env_dotenv.nim +++ b/ModularBot_garage/src/env_dotenv.nim @@ -23,6 +23,23 @@ ## ## The boot report (env_report.nim) reads the resolved path/source and the set ## of keys that came from the file, so it can label values `(source: .env)`. +## +## INLINE COMMENTS (j139): a `#` preceded by WHITESPACE and not inside quotes +## starts a comment that runs to the end of the line. Rule, in order, on the +## text after the `=`: +## * the value is scanned left to right, tracking single/double quotes; the +## first unquoted `#` that is either at position 0 or preceded by +## whitespace ends the value (everything from there is dropped, then the +## value is trimmed again); +## * a `#` INSIDE quotes is data (`"a # b"` -> `a # b`); +## * a `#` with no whitespace before it is data (`value#tag` stays), because +## a `#` glued to the value is far more likely to be part of a token than +## a comment, and guessing wrong there would silently corrupt data; +## * `KEY=# comment` yields the empty value, not an error. +## +## Because readers fall back to their DEFAULT on any value they do not +## recognise, a value that still looks junk after parsing is warned about at +## load time (one line per key) instead of being applied silently. import std/[os, strutils, sets] @@ -97,6 +114,32 @@ proc chooseEnvFile*(flagValue, envValue, cwdCandidate, exeCandidate: string): En # ── parsing ────────────────────────────────────────────────────────────────── +proc stripInlineComment*(raw: string): string = + ## Pure, exported for the guard test: drop a trailing ` # comment` that is + ## not inside quotes. See the module doc comment for the full rule. + var quote = ' ' + for i, c in raw: + if c == '"' or c == '\'': + if quote == ' ': quote = c + elif quote == c: quote = ' ' + elif c == '#' and quote == ' ' and (i == 0 or raw[i - 1] in Whitespace): + return raw[0 ..< i] + return raw + +proc looksJunky*(value: string): bool = + ## True when a value is NOT a plain token: it still contains whitespace or a + ## `#`, or a quote that was never closed. Legitimate values (comma lists like + ## `1.0,1.25`, paths with `/`, `both`, `off`, `0`) are never flagged. + if value.len == 0: return false + if value.contains('#'): return true + var dq = 0 + var sq = 0 + for c in value: + if c == '"': inc dq + elif c == '\'': inc sq + elif c in Whitespace: return true + dq mod 2 == 1 or sq mod 2 == 1 + proc parseEnvFileContent*(content, filename: string): seq[EnvEntry] = ## Parse the usual .env shape: one KEY=VALUE per line, blank lines and `#` ## comments skipped, a leading `export ` tolerated, surrounding single or @@ -123,7 +166,7 @@ proc parseEnvFileContent*(content, filename: string): seq[EnvEntry] = if key.len == 0: raise newException(ValueError, filename & ":" & $lineno & ": empty key in: " & stripped) - var value = body[eq + 1 .. ^1].strip() + var value = stripInlineComment(body[eq + 1 .. ^1]).strip() if value.len >= 2 and ((value[0] == '"' and value[^1] == '"') or (value[0] == '\'' and value[^1] == '\'')): @@ -132,6 +175,18 @@ proc parseEnvFileContent*(content, filename: string): seq[EnvEntry] = # ── applying ───────────────────────────────────────────────────────────────── +proc junkWarnings*(entries: seq[EnvEntry], path: string): seq[string] = + ## The one-line-per-key warnings for values that are not plain tokens. Pure + ## (no printing) so the guard test can pin the text; `applyEnvFile` prints + ## exactly these lines. An offending value is still applied verbatim, but any + ## reader that does not recognise it falls back to its DEFAULT — the message + ## says so and tells the user to move the comment onto its own line. + for e in entries: + if looksJunky(e.value): + result.add "[dotenv] WARNING: " & path & ": " & e.key & + " raw value looks wrong: \"" & e.value & "\"" & + " - the reader will fall back to its DEFAULT; move the comment onto its own line" + proc applyEnvFile*(path: string): seq[EnvConflict] = ## Read `path`, apply every entry with putEnv (the FILE WINS over the real ## environment), remember the from-file keys, and return the keys whose file @@ -145,6 +200,10 @@ proc applyEnvFile*(path: string): seq[EnvConflict] = result.add EnvConflict(key: e.key, fileValue: e.value, shellValue: shell) putEnv(e.key, e.value) gEnvFileKeys.incl e.key + # j139 safety net: a value that is not a plain token will be rejected by the + # reader (which then falls back to its DEFAULT, silently). Say so, once. + for w in junkWarnings(entries, path): + stderr.writeLine w proc loadEnvFile*(choice: EnvFileChoice): seq[EnvConflict] = ## Apply the chosen file. A missing EXPLICIT file raises EnvFileError; a diff --git a/common_libs/tests/test_env_dotenv.nim b/common_libs/tests/test_env_dotenv.nim index 3382fe9..0b2f2d0 100644 --- a/common_libs/tests/test_env_dotenv.nim +++ b/common_libs/tests/test_env_dotenv.nim @@ -10,6 +10,8 @@ ## * resolution precedence (flag > TR_ENV_FILE > ./.env > .env next to the ## executable > none); ## * the FILE WINS over the real environment, and the conflict is reported; +## * a junky value (unrecognised) is WARNED ABOUT at load time instead of +## silently becoming the reader's default (j139); ## * an explicitly requested missing file raises (the caller then exits ## nonzero), while a missing DEFAULT file is a silent no-op. @@ -166,12 +168,76 @@ proc testMissingFiles() = check "no file at all is a silent no-op", loadEnvFile(EnvFileChoice(path: "", source: "none", explicit: false)).len == 0 +# ── 5. inline comments (j139) and the junk-value warning ───────────────────── + +proc testInlineComments() = + let e = parseEnvFileContent( + "A=value # comment\n" & + "B=\"a # b\"\n" & + "C='a # b'\n" & + "D=value#notacomment\n" & + "E=# comment\n" & + "# whole line\n" & + "F=0 # hides the heat grid\n" & + "G=both\n", "inline.env") + check "inline comment file yields 7 entries", e.len == 7 + if e.len == 7: + check "`KEY=value # comment` -> value", e[0].key == "A" and e[0].value == "value" + check "a `#` inside double quotes is DATA", e[1].value == "a # b" + check "a `#` inside single quotes is DATA", e[2].value == "a # b" + check "`KEY=value#notacomment` keeps the `#` (no whitespace before it)", + e[3].value == "value#notacomment" + check "`KEY=# comment` -> empty value (not an error)", e[4].value == "" + check "the reported bug shape `0 # comment` -> `0`", e[5].value == "0" + check "a line whose only content is a comment is still skipped", e[6].value == "both" + + check "stripInlineComment is pure and exact", + stripInlineComment("x # y").strip == "x" and + stripInlineComment(" # y").strip == "" and + stripInlineComment("\"a # b\"").strip == "\"a # b\"" + +proc testJunkWarning() = + check "a plain token is never junky", + not looksJunky("0") and not looksJunky("off") and + not looksJunky("1.0,1.25") and + not looksJunky("/tmp/modularbot.log") and not looksJunky("") + check "whitespace / `#` / unbalanced quotes are junky", + looksJunky("0 # hides the grid") and looksJunky("a # b") and + looksJunky("\"unclosed") + + # A junky value the inline-comment rule CANNOT fix: an unclosed quote, so + # the `#` is (correctly) treated as data and the whole tail stays in the value. + let f = writeTmp("junk.env", "TR_TEST_JUNK_J139=\"0 # hides the grid\n") + let conflicts = applyEnvFile(f) + check "a junky value is still APPLIED verbatim (the reader decides)", + conflicts.len == 0 and + getEnv("TR_TEST_JUNK_J139") == "\"0 # hides the grid" + delEnv("TR_TEST_JUNK_J139") + + # the warning path itself: one line per offending key, naming file, key, + # raw value and what is used. + let w = junkWarnings(parseEnvFileContent(readFile(f), f), f) + check "a junky value produces exactly one warning", + w.len == 1 + check "the warning names the file", w.len == 1 and f in w[0] + check "the warning names the key and the raw value", + w.len == 1 and "TR_TEST_JUNK_J139" in w[0] and "# hides the grid" in w[0] + check "the warning says what is used and how to fix it", + w.len == 1 and "DEFAULT" in w[0] and "its own line" in w[0] + check "a clean file produces no warning", + junkWarnings(parseEnvFileContent("A=0 # fine\nB=1.0,1.25\n", "c.env"), + "c.env").len == 0 + check "one line per key, no spam", + junkWarnings(parseEnvFileContent("A=x y\nB=p q\n", "d.env"), "d.env").len == 2 + # ── driver ─────────────────────────────────────────────────────────────────── testParser() testResolution() testFileWins() testMissingFiles() +testInlineComments() +testJunkWarning() echo "\n", checks, " checks, ", failures, " failure(s)" if failures > 0: diff --git a/docs/env_reference.md b/docs/env_reference.md index c06e81b..eb94144 100644 --- a/docs/env_reference.md +++ b/docs/env_reference.md @@ -8,7 +8,9 @@ surprise you. 1. In the bot folder, copy `.env.example` to `.env`. 2. Open `.env` and set the knobs you want. One `KEY=VALUE` per line. Lines - starting with `#` are comments. + starting with `#` are comments. A trailing comment is allowed and stripped + (`KEY=0 # off`) as long as the `#` is outside quotes; a value that still + contains whitespace or a `#` after that gets one `[dotenv] WARNING` line. 3. Start the bot from the bot folder (`./ModularBot`). It picks up `.env` automatically. 4. To use a different file, pass it on the command line: