Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #17853 (ascii message separator broke json nim dump) #17887

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
3 changes: 2 additions & 1 deletion compiler/main.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
1 change: 1 addition & 0 deletions compiler/msgs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ proc `??`* (conf: ConfigRef; info: TLineInfo, filename: string): bool =

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
Expand Down
2 changes: 1 addition & 1 deletion compiler/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type # please make sure we have under 32 options
optImportHidden

TOptions* = set[TOption]
TGlobalOption* = enum # **keep binary compatible**
Araq marked this conversation as resolved.
Show resolved Hide resolved
TGlobalOption* = enum
gloptNone, optForceFullMake,
optWasNimscript, # redundant with `cmdNimscript`, could be removed
optListCmd, optCompileOnly, optNoLinking,
Expand Down
11 changes: 11 additions & 0 deletions tests/misc/trunner.nim
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ ret=[s1:foobar s2:foobar age:25 pi:3.14]

else: # don't run twice the same test
import std/strutils
import std/json
template check2(msg) = doAssert msg in output, output

block: # tests with various options `nim doc --project --index --docroot`
Expand Down Expand Up @@ -317,3 +318,13 @@ compiling: v3
running: v3
running: v2
""", ret

block: # nim dump
let cmd = fmt"{nim} dump --dump.format:json -d:D20210428T161003 --hints:off ."
let (ret, status) = execCmdEx(cmd)
doAssert status == 0
let j = ret.parseJson
# sanity checks
doAssert "D20210428T161003" in j["defined_symbols"].to(seq[string])
doAssert j["version"].to(string) == NimVersion
doAssert j["nimExe"].to(string) == getCurrentCompilerExe()
7 changes: 5 additions & 2 deletions tests/osproc/treadlines.nim
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
discard """
output: '''Error: cannot open 'a.nim'\31
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()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous code was using "nim", which didn't honor testament --nim:/pathto/nim r test

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})

Expand Down