Skip to content

Commit 5a5e2a4

Browse files
elcritchYour Name
andauthored
UX and Test Improvements (#1314)
* add progress spinner for terminals * tweak spinner spacing * fix nimble deps printing and tweak format * fix nimble deps printing and tweak format * update deps human formatting * update deps human formatting * update deps human formatting * update deps human formatting * handle no colors in deps human formatting * handle no colors in deps human formatting * handle no colors in deps human formatting * handle no colors in deps human formatting * handle no colors in deps human formatting * rename levelSkips to levelInfos for clarity * add format info * add inverted * add inverted * add inverted * add inverted * add inverted * add inverted * add inverted * cleanup * merge changes * change inverted deps format * change inverted deps format * change inverted deps format * change inverted deps format * sorting deps output * tweak outputs * tweak outputs * tweak outputs * fix unused imports * fix unused imports * fix json deps * honor SilentPriority with progress spinner * shore up odd test behavior * ensure clean start * catch windows OSError from eraseLine * reset the conflictingdepres test dir * reset the conflictingdepres test dir * execNimble add --noColor by default for testing * fix accidental change * reset the conflictingdepres test dir * ignore official-nim-releases.json * tweak output to handle non-nimble messages better * don't override nim metadata when the json file is missing and report mismatching nim version / pkg info versions --------- Co-authored-by: Your Name <[email protected]>
1 parent 123f97a commit 5a5e2a4

File tree

11 files changed

+190
-21
lines changed

11 files changed

+190
-21
lines changed

src/nimble.nim

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,9 +1944,10 @@ proc depsTree(options: Options) =
19441944

19451945
if options.action.format == "json":
19461946
echo (%depsRecursive(pkgInfo, dependencies, errors)).pretty
1947+
elif options.action.format == "inverted":
1948+
printDepsHumanReadableInverted(pkgInfo, dependencies, errors)
19471949
else:
1948-
echo pkgInfo.basicInfo.name
1949-
printDepsHumanReadable(pkgInfo, dependencies, 1, errors)
1950+
printDepsHumanReadable(pkgInfo, dependencies, errors)
19501951

19511952
proc syncWorkingCopy(name: string, path: Path, dependentPkg: PackageInfo,
19521953
options: Options) =
@@ -2511,4 +2512,5 @@ when isMainModule:
25112512
displayError(&"Couldn't save \"{nimbleDataFileName}\".")
25122513
displayDetails(error)
25132514

2515+
displayLineReset()
25142516
quit(exitCode)

src/nimblepkg/cli.nim

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ type
3030
DebugPriority, LowPriority, MediumPriority, HighPriority, SilentPriority
3131

3232
DisplayType* = enum
33-
Error, Warning, Details, Hint, Message, Success
33+
Error, Warning, Details, Hint, Message, Success, Progress
3434

3535
ForcePrompt* = enum
3636
dontForcePrompt, forcePromptYes, forcePromptNo
3737

3838
const
3939
longestCategory = len("Downloading")
40-
foregrounds: array[Error .. Success, ForegroundColor] =
41-
[fgRed, fgYellow, fgBlue, fgWhite, fgCyan, fgGreen]
40+
foregrounds: array[Error .. Progress, ForegroundColor] =
41+
[fgRed, fgYellow, fgBlue, fgWhite, fgCyan, fgGreen, fgMagenta]
4242
styles: array[DebugPriority .. HighPriority, set[Style]] =
4343
[{styleDim}, {styleDim}, {}, {styleBright}]
4444

@@ -61,6 +61,13 @@ proc isSuppressed(displayType: DisplayType): bool =
6161
globalCLI.level == HighPriority:
6262
return true
6363

64+
proc displayFormatted*(displayType: DisplayType, msgs: varargs[string]) =
65+
for msg in msgs:
66+
if globalCLI.showColor:
67+
stdout.styledWrite(foregrounds[displayType], msg)
68+
else:
69+
stdout.write(msg)
70+
6471
proc displayCategory(category: string, displayType: DisplayType,
6572
priority: Priority) =
6673
if isSuppressed(displayType):
@@ -80,15 +87,40 @@ proc displayCategory(category: string, displayType: DisplayType,
8087
else:
8188
stdout.write(text)
8289

90+
const
91+
spinChars = ["","","","","","","",""]
92+
var
93+
lastWasDot = false
94+
lastCharidx = 0
95+
96+
proc displayLineReset*() =
97+
if lastWasDot:
98+
try:
99+
stdout.cursorUp(1)
100+
stdout.eraseLine()
101+
except OSError:
102+
discard # this breaks on windows a lot so we ignore it
103+
lastWasDot = false
104+
83105
proc displayLine(category, line: string, displayType: DisplayType,
84106
priority: Priority) =
107+
displayLineReset()
108+
85109
if isSuppressed(displayType):
110+
stdout.write "+"
86111
return
87112

88113
displayCategory(category, displayType, priority)
89114

90115
# Display the message.
91-
echo(line)
116+
if displayType != Progress:
117+
echo(line)
118+
else:
119+
# displayCategory("Executing", Warning, HighPriority)
120+
stdout.write(spinChars[lastCharidx], " ", line, "\n")
121+
lastCharidx = (lastCharidx + 1) mod spinChars.len()
122+
stdout.flushFile()
123+
lastWasDot = true
92124

93125
proc display*(category, msg: string, displayType = Message,
94126
priority = MediumPriority) =
@@ -105,9 +137,18 @@ proc display*(category, msg: string, displayType = Message,
105137
if priority < globalCLI.level:
106138
if priority != DebugPriority:
107139
globalCLI.suppressionCount.inc
140+
if globalCLI.showColor and globalCLI.level != SilentPriority:
141+
# some heuristics here
142+
if category == "Executing" and msg.endsWith("printPkgInfo"):
143+
displayLine("Scanning", "", Progress, HighPriority)
144+
elif msg.startsWith("git"):
145+
displayLine("Updating", "", Progress, HighPriority)
146+
else:
147+
displayLine("Working", "", Progress, HighPriority)
108148
return
109149

110150
# Display each line in the message.
151+
111152
var i = 0
112153
for line in msg.splitLines():
113154
if len(line) == 0: continue

src/nimblepkg/deps.nim

Lines changed: 114 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import packageinfotypes, developfile, packageinfo, version, tables, strformat, strutils
1+
import tables, strformat, sequtils, algorithm, cli
2+
import packageinfotypes, developfile, packageinfo, version
23

34
type
45
DependencyNode = ref object of RootObj
@@ -33,20 +34,122 @@ proc depsRecursive*(pkgInfo: PackageInfo,
3334

3435
proc printDepsHumanReadable*(pkgInfo: PackageInfo,
3536
dependencies: seq[PackageInfo],
36-
level: int,
37-
errors: ValidationErrors) =
37+
errors: ValidationErrors,
38+
levelInfos: seq[tuple[skip: bool]] = @[]
39+
) =
40+
## print human readable tree deps
41+
##
42+
if levelInfos.len() == 0:
43+
displayLineReset()
44+
displayInfo("Dependency tree format: {PackageName} {Requirements} (@{Resolved Version})")
45+
displayFormatted(Hint, "\n")
46+
displayFormatted(Message, pkgInfo.basicInfo.name, " ")
47+
displayFormatted(Success, "(@", $pkgInfo.basicInfo.version, ")")
48+
49+
var requires: seq[(string, VersionRange, bool, PackageInfo)]
50+
for idx, (name, ver) in pkgInfo.requires.sorted():
51+
var depPkgInfo = initPackageInfo()
52+
let
53+
found = dependencies.findPkg((name, ver), depPkgInfo)
54+
packageName = if found: depPkgInfo.basicInfo.name else: name
55+
requires.add((packageName, ver, found, depPkgInfo))
56+
57+
proc reqCmp[T](x, y: T): int = cmp(x[0], y[0])
58+
requires.sort(reqCmp)
59+
60+
for idx, (packageName, ver, found, depPkgInfo) in requires:
61+
let
62+
isLast = idx == pkgInfo.requires.len() - 1
63+
64+
displayFormatted(Hint, "\n")
65+
for levelInfo in levelInfos:
66+
if levelInfo.skip:
67+
displayFormatted(Hint, " ")
68+
else:
69+
displayFormatted(Hint, "")
70+
if not isLast:
71+
displayFormatted(Hint, "├── ")
72+
else:
73+
displayFormatted(Hint, "└── ")
74+
displayFormatted(Message, packageName)
75+
displayFormatted(Hint, " ")
76+
displayFormatted(Warning, if ver.kind == verAny: "@any" else: $ver)
77+
displayFormatted(Hint, " ")
78+
if found:
79+
displayFormatted(Success, fmt"(@{depPkgInfo.basicInfo.version})")
80+
if errors.contains(packageName):
81+
let errMsg = getValidationErrorMessage(packageName, errors.getOrDefault packageName)
82+
displayFormatted(Error, fmt" - error: {errMsg}")
83+
if found:
84+
var levelInfos = levelInfos & @[(skip: isLast)]
85+
printDepsHumanReadable(depPkgInfo, dependencies, errors, levelInfos)
86+
if levelInfos.len() == 0:
87+
displayFormatted(Hint, "\n")
88+
89+
proc printDepsHumanReadableInverted*(pkgInfo: PackageInfo,
90+
dependencies: seq[PackageInfo],
91+
errors: ValidationErrors,
92+
pkgs = newTable[string, TableRef[string, VersionRange]](),
93+
) =
94+
## print human readable tree deps
95+
##
96+
let
97+
parent = pkgInfo.basicInfo.name
98+
isRoot = pkgs.len() == 0
99+
100+
if isRoot:
101+
displayInfo("Dependency tree format: {PackageName} (@{Resolved Version})")
102+
displayInfo("Dependency tree format: {Source Package} {Source Requirements}")
103+
displayFormatted(Hint, "\n")
104+
displayFormatted(Message, pkgInfo.basicInfo.name, " ")
105+
displayFormatted(Success, "(@", $pkgInfo.basicInfo.version, ")")
106+
displayFormatted(Hint, "\n")
107+
38108
for (name, ver) in pkgInfo.requires:
39109
var depPkgInfo = initPackageInfo()
40110
let
41111
found = dependencies.findPkg((name, ver), depPkgInfo)
42112
packageName = if found: depPkgInfo.basicInfo.name else: name
43113

44-
echo " ".repeat(level * 2),
45-
packageName,
46-
if ver.kind == verAny: "@any" else: " " & $ver,
47-
if found: fmt "(resolved {depPkgInfo.basicInfo.version})" else: "",
48-
if errors.contains(packageName):
49-
" - error: " & getValidationErrorMessage(packageName, errors.getOrDefault packageName)
114+
pkgs.mgetOrPut(packageName, newTable[string, VersionRange]())[parent] = ver
115+
116+
if found:
117+
printDepsHumanReadableInverted(depPkgInfo, dependencies, errors, pkgs)
118+
119+
if isRoot:
120+
# for pkg, info in pkgs:
121+
for idx, name in pkgs.keys().toSeq().sorted():
122+
let
123+
info = pkgs[name]
124+
isOuterLast = idx == pkgs.len() - 1
125+
if not isOuterLast:
126+
displayFormatted(Hint, "├── ")
50127
else:
51-
""
52-
if found: printDepsHumanReadable(depPkgInfo, dependencies, level + 1, errors)
128+
displayFormatted(Hint, "└── ")
129+
displayFormatted(Message, name, " ")
130+
displayFormatted(Success, "(@", $pkgInfo.basicInfo.version, ")")
131+
displayFormatted(Hint, "\n")
132+
# echo "name: ", pkg, " info: ", $info
133+
# for idx, (source, ver) in info.pairs().toSeq():
134+
proc printOuter() =
135+
if not isOuterLast:
136+
displayFormatted(Hint, "")
137+
else:
138+
displayFormatted(Hint, " ")
139+
for idx, source in info.keys().toSeq().sorted():
140+
let
141+
ver = info[source]
142+
isLast = idx == info.len() - 1
143+
144+
if not isLast:
145+
printOuter()
146+
displayFormatted(Hint, "╟── ")
147+
else:
148+
printOuter()
149+
displayFormatted(Hint, "╙── ")
150+
displayFormatted(Warning, if ver.kind == verAny: "@any" else: $ver)
151+
displayFormatted(Hint, " ")
152+
displayFormatted(Message, source)
153+
displayFormatted(Hint, "\n")
154+
displayFormatted(Hint, "\n")
155+

src/nimblepkg/downloadnim.nim

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,8 @@ proc installNimFromBinariesDir*(
785785
isNimDirProperlyExtracted(pkg.getRealDir):
786786
let ver = getNimVersion(pkg.getRealDir)
787787
if ver.isSome():
788+
if pkg.basicInfo.version != ver.get():
789+
displayWarning("Nim binary version doesn't match the package info version for Nim located at: " & pkg.getRealDir)
788790
return some (pkg.getRealDir, ver.get())
789791

790792
# Download if allowed

src/nimblepkg/nimscriptwrapper.nim

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ proc execNimscript(
7070
cmd &= " " & i.quoteShell()
7171
cmd &= " " & join(options.action.custRunFlags, " ")
7272

73-
displayDebug("Executing " & cmd)
73+
displayDebug("Executing", cmd)
7474

7575
if needsLiveOutput(actionName, options, isHook):
7676
result.exitCode = execCmd(cmd)

src/nimblepkg/packageinfo.nim

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,10 @@ proc getInstalledPackageMin*(options: Options, pkgDir, nimbleFilePath: string):
298298
setNameVersionChecksum(result, pkgDir)
299299
result.isMinimal = true
300300
result.isInstalled = true
301-
fillMetaData(result, pkgDir, false, options)
301+
try:
302+
fillMetaData(result, pkgDir, true, options)
303+
except MetaDataError:
304+
discard
302305

303306
proc getInstalledPkgsMin*(libsDir: string, options: Options): seq[PackageInfo] =
304307
## Gets a list of installed packages. The resulting package info is

tests/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
/issue793/src/htmldocs/
3838
/localdeps/localdeps
3939
/localdeps/nimbledeps/
40+
/official-nim-releases.json
4041

4142
# occurs on multiple levels
4243
nimbleDir/

tests/tdeps.nim

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ suite "nimble deps":
1414
check exitCode == QuitSuccess
1515
check output.contains("""
1616
deps
17-
timezones 0.5.4(resolved 0.5.4)
17+
timezones 0.5.4 (resolved 0.5.4)
1818
nim >= 0.19.9
1919
""")
2020

tests/testscommon.nim

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ proc execNimble*(args: varargs[string]): ProcessOutput =
3131
var quotedArgs = @args
3232
if not args.anyIt("--nimbleDir:" in it or "-l" == it or "--local" == it):
3333
quotedArgs.insert("--nimbleDir:" & installDir)
34+
quotedArgs.insert("--noColor")
3435
quotedArgs.insert(nimblePath)
3536
quotedArgs = quotedArgs.map((x: string) => x.quoteShell)
3637

tests/tlocaldeps.nim

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ import testscommon
88
from nimblepkg/common import cd
99

1010
suite "project local deps mode":
11+
setup:
12+
# do this to prevent non-deterministic (random?) setting of the NIMBLE_DIR
13+
# which messes up this test sometime
14+
delEnv("NIMBLE_DIR")
15+
1116
test "nimbledeps exists":
1217
cd "localdeps":
1318
cleanDir("nimbledeps")
@@ -26,9 +31,11 @@ suite "project local deps mode":
2631
check output.contains("Succeeded")
2732

2833
test "localdeps develop":
34+
cleanDir("nimbledeps")
2935
cleanDir("packagea")
3036
let (_, exitCode) = execCmdEx(nimblePath &
3137
&" develop {pkgAUrl} --localdeps -y")
38+
discard execCmd("find packagea")
3239
check exitCode == QuitSuccess
3340
check dirExists("packagea" / "nimbledeps")
3441
check not dirExists("nimbledeps")

0 commit comments

Comments
 (0)