address comment

changelog.md

@@ -60,6 +60,10 @@
 
 - Removed `.travis.yml`, `appveyor.yml.disabled`, `.github/workflows/ci.yml.disabled`.
 
+- Nim compiler now adds ASCII unit separator `\31` before a newline for every generated
+  message (potentially multiline), so tooling can tell when messages start and end.
+
+
 ## Standard library additions and changes
 - Added support for parenthesized expressions in `strformat`
 
@@ -378,9 +382,6 @@
 
 - `--hint:CC` now goes to stderr (like all other hints) instead of stdout.
 
-- Added `--msgSep:on|off` to add ASCII unit separator before a newline for every end of any kind of message,
-  so tooling can tell when messages start and end.
-
 - json build instructions are now generated in `$nimcache/outFileBasename.json`
   instead of `$nimcache/projectName.json`. This allows avoiding recompiling a given project
   compiled with different options if the output file differs.

compiler/commands.nim

@@ -894,8 +894,6 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
     trackIde(conf, ideDus, arg, info)
   of "stdout":
     processOnOffSwitchG(conf, {optStdout}, arg, pass, info)
-  of "msgsep":
-    processOnOffSwitchG(conf, {optMsgSep}, arg, pass, info)
   of "filenames":
     case arg.normalize
     of "abs": conf.filenameOption = foAbs

compiler/main.nim

@@ -349,7 +349,8 @@ proc mainCommand*(graph: ModuleGraph) =
         (key: "warnings", val: warnings),
       ]
 
-      msgWriteln(conf, $dumpdata, {msgStdout, msgSkipHook})
+      msgWriteln(conf, $dumpdata, {msgStdout, msgSkipHook, msgNoUnitSep})
+        # `msgNoUnitSep` to avoid generating invalid json, refs bug #17853
     else:
       msgWriteln(conf, "-- list of currently defined symbols --",
                  {msgStdout, msgSkipHook, msgNoUnitSep})

compiler/msgs.nim

@@ -289,16 +289,17 @@ proc `??`* (conf: ConfigRef; info: TLineInfo, filename: string): bool =
   # only for debugging purposes
   result = filename in toFilename(conf, info)
 
+const
+  UnitSep = "\31"
+    # this needs care to avoid issues similar to https://github.com/nim-lang/Nim/issues/17853
+
 type
   MsgFlag* = enum  ## flags altering msgWriteln behavior
     msgStdout,     ## force writing to stdout, even stderr is default
     msgSkipHook    ## skip message hook even if it is present
     msgNoUnitSep  ## the message is a complete "paragraph".
   MsgFlags* = set[MsgFlag]
 
-proc msgSep*(conf: ConfigRef): string {.inline.} =
-  if optMsgSep in conf.globalOptions: "\31" else: ""
-
 proc msgWriteln*(conf: ConfigRef; s: string, flags: MsgFlags = {}) =
   ## Writes given message string to stderr by default.
   ## If ``--stdout`` option is given, writes to stdout instead. If message hook
@@ -308,7 +309,7 @@ proc msgWriteln*(conf: ConfigRef; s: string, flags: MsgFlags = {}) =
   ## This is used for 'nim dump' etc. where we don't have nimsuggest
   ## support.
   #if conf.cmd == cmdIdeTools and optCDebug notin gGlobalOptions: return
-  let sep = if msgNoUnitSep notin flags: conf.msgSep else: ""
+  let sep = if msgNoUnitSep notin flags: UnitSep else: ""
   if not isNil(conf.writelnHook) and msgSkipHook notin flags:
     conf.writelnHook(s & sep)
   elif optStdout in conf.globalOptions or msgStdout in flags:
@@ -408,7 +409,7 @@ proc quit(conf: ConfigRef; msg: TMsgKind) {.gcsafe.} =
         styledMsgWriteln(fgRed, """
 No stack traceback available
 To create a stacktrace, rerun compilation with './koch temp $1 <file>', see $2 for details""" %
-          [conf.command, "intern.html#debugging-the-compiler".createDocLink], conf.msgSep)
+          [conf.command, "intern.html#debugging-the-compiler".createDocLink], UnitSep)
   quit 1
 
 proc handleError(conf: ConfigRef; msg: TMsgKind, eh: TErrorHandling, s: string) =
@@ -549,13 +550,13 @@ proc liMessage*(conf: ConfigRef; info: TLineInfo, msg: TMsgKind, arg: string,
         msgWrite(conf, ".")
       else:
         styledMsgWriteln(styleBright, loc, resetStyle, color, title, resetStyle, s, KindColor, kindmsg,
-                         resetStyle, conf.getSurroundingSrc(info), conf.msgSep)
+                         resetStyle, conf.getSurroundingSrc(info), UnitSep)
         if hintMsgOrigin in conf.mainPackageNotes:
           # xxx needs a bit of refactoring to honor `conf.filenameOption`
           styledMsgWriteln(styleBright, toFileLineCol(info2), resetStyle,
             " compiler msg initiated here", KindColor,
             KindFormat % $hintMsgOrigin,
-            resetStyle, conf.msgSep)
+            resetStyle, UnitSep)
   handleError(conf, msg, eh, s)
 
 template rawMessage*(conf: ConfigRef; msg: TMsgKind, args: openArray[string]) =

compiler/options.nim

@@ -66,7 +66,6 @@ type                          # please make sure we have under 32 options
     optUseColors,             # use colors for hints, warnings, and errors
     optThreads,               # support for multi-threading
     optStdout,                # output to stdout
-    optMsgSep,                # use message separator for IDEs
     optThreadAnalysis,        # thread analysis pass
     optTlsEmulation,          # thread var emulation turned on
     optGenIndex               # generate index file for documentation;

doc/advopt.txt

@@ -35,8 +35,6 @@ Advanced options:
   --usenimcache             will use `outdir=$$nimcache`, whichever it resolves
                             to after all options have been processed
   --stdout:on|off           output to stdout
-  --msgSep:on|off           insert ASCII unit separator `\31` after each
-                            compiler message for IDE support.
   --colors:on|off           turn compiler messages coloring on|off
   --filenames:abs|canonical|legacyRelProj
                             customize how filenames are rendered in compiler messages,

tests/misc/trunner.nim

@@ -238,15 +238,15 @@ tests/newconfig/bar/mfoo.nims""".splitLines
     var expected = ""
     for a in files:
       let b = dir / a
-      expected.add &"Hint: used config file '{b}' [Conf]\n"
+      expected.add &"Hint: used config file '{b}' [Conf]\31\n"
     doAssert outp.endsWith expected, outp & "\n" & expected
 
   block: # mfoo2.customext
     let filename = testsDir / "newconfig/foo2/mfoo2.customext"
     let cmd = fmt"{nim} e --hint:conf {filename}"
     let (outp, exitCode) = execCmdEx(cmd, options = {poStdErrToStdOut})
     doAssert exitCode == 0
-    var expected = &"Hint: used config file '{filename}' [Conf]\n"
+    var expected = &"Hint: used config file '{filename}' [Conf]\31\n"
     doAssert outp.endsWith "123" & "\n" & expected
 
 

tests/osproc/treadlines.nim

@@ -1,17 +1,15 @@
 discard """
-  output: '''Error: cannot open 'a.nim'
-Error: cannot open 'b.nim'
+  output: '''Error: cannot open 'a.nim'\31
+Error: cannot open 'b.nim'\31
 '''
   targets: "c"
 """
 
 import osproc
-from std/os import getCurrentCompilerExe
 
 var ps: seq[Process] # compile & run 2 progs in parallel
-const nim = getCurrentCompilerExe()
 for prog in ["a", "b"]:
-  ps.add startProcess(nim, "",
+  ps.add startProcess("nim", "",
                       ["r", "--hint[Conf]=off", "--hint[Processing]=off", prog],
                       options = {poUsePath, poDaemon, poStdErrToStdOut})