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.

Sign up for GitHub

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 cli invocation from nimscript #109

Merged
merged 4 commits into from
Oct 30, 2024
Merged

Fix cli invocation from nimscript #109

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9fd8d37
Fix `cli` invocation from nimscript
etan-status Oct 28, 2024
5d1a5de
Add debugging output on failure
etan-status Oct 28, 2024
2203902
Update confutils.nimble
etan-status Oct 28, 2024
1a4ccec
Update confutils.nim
etan-status Oct 30, 2024
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
119 changes: 60 additions & 59 deletions confutils.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,29 @@
# This file may not be copied, modified, or distributed except according to
# those terms.

{.push raises: [].}

import
os,
std/[enumutils, options, strutils, wordwrap, strformat],
std/[enumutils, options, strutils, wordwrap],
stew/shims/macros,
serialization,
confutils/[defs, cli_parser, config_file]

export
options, serialization, defs, config_file
options, defs, config_file

const
hasSerialization = not defined(nimscript)
useBufferedOutput = defined(nimscript)
noColors = useBufferedOutput or defined(confutils_no_colors)
hasCompletions = not defined(nimscript)
descPadding = 6
minNameWidth = 24 - descPadding

when hasSerialization:
import serialization
export serialization

when not defined(nimscript):
import
terminal,
Expand Down Expand Up @@ -420,10 +426,7 @@ func getNextArgIdx(cmd: CmdInfo, consumedArgIdx: int): int =
proc noMoreArgsError(cmd: CmdInfo): string {.raises: [].} =
result =
if cmd.isSubCommand:
try:
"The command '$1'" % [cmd.name]
except ValueError as err:
raiseAssert "strutils.`%` failed: " & err.msg
"The command '" & cmd.name & "'"
else:
appInvocation()
result.add " does not accept"
Expand Down Expand Up @@ -630,12 +633,14 @@ proc completeCmdArgAux(T: type, val: string): seq[string] =
mixin completeCmdArg
return completeCmdArg(T, val)

template setField[T](loc: var T, val: Option[string], defaultVal: untyped) =
template setField[T](
loc: var T, val: Option[string], defaultVal: untyped): untyped =
type FieldType = type(loc)
loc = if isSome(val): parseCmdArgAux(FieldType, val.get)
else: FieldType(defaultVal)

template setField[T](loc: var seq[T], val: Option[string], defaultVal: untyped) =
template setField[T](
loc: var seq[T], val: Option[string], defaultVal: untyped): untyped =
if val.isSome:
loc.add parseCmdArgAux(type(loc[0]), val.get)
else:
Expand Down Expand Up @@ -867,36 +872,34 @@ macro configurationRtti(RecordType: type): untyped =

result = newTree(nnkPar, newLitFixed cmdInfo, fieldSetters)

proc addConfigFile*(secondarySources: auto,
Format: type,
path: InputFile) {.raises: [ConfigurationError].} =
try:
secondarySources.data.add loadFile(Format, string path,
when hasSerialization:
proc addConfigFile*(secondarySources: auto,
Format: type,
path: InputFile) {.raises: [ConfigurationError].} =
try:
secondarySources.data.add loadFile(Format, string path,
type(secondarySources.data[0]))
except SerializationError as err:
raise newException(ConfigurationError, err.formatMsg(string path), err)
except IOError as err:
raise newException(ConfigurationError,
"Failed to read config file at '" & string(path) & "': " & err.msg)

proc addConfigFileContent*(secondarySources: auto,
Format: type,
content: string) {.raises: [ConfigurationError].} =
try:
secondarySources.data.add decode(Format, content,
type(secondarySources.data[0]))
except SerializationError as err:
raise newException(ConfigurationError, err.formatMsg(string path), err)
except IOError as err:
raise newException(ConfigurationError,
"Failed to read config file at '" & string(path) & "': " & err.msg)

proc addConfigFileContent*(secondarySources: auto,
Format: type,
content: string) {.raises: [ConfigurationError].} =
try:
secondarySources.data.add decode(Format, content,
type(secondarySources.data[0]))
except SerializationError as err:
raise newException(ConfigurationError, err.formatMsg("<content>"), err)
except IOError:
raiseAssert "This should not be possible"
except SerializationError as err:
raise newException(ConfigurationError, err.formatMsg("<content>"), err)
except IOError:
raiseAssert "This should not be possible"

func constructEnvKey*(prefix: string, key: string): string {.raises: [].} =
## Generates env. variable names from keys and prefix following the
## IEEE Open Group env. variable spec: https://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html
try:
(&"{prefix}_{key}").toUpperAscii.multiReplace(("-", "_"), (" ", "_"))
except ValueError as err:
raiseAssert "strformat.`&` failed: " & err.msg
(prefix & "_" & key).toUpperAscii.multiReplace(("-", "_"), (" ", "_"))

# On Posix there is no portable way to get the command
# line from a DLL and thus the proc isn't defined in this environment.
Expand All @@ -915,7 +918,7 @@ proc loadImpl[C, SecondarySources](
secondarySources: proc (
config: Configuration, sources: ref SecondarySources
) {.gcsafe, raises: [ConfigurationError].} = nil,
envVarsPrefix = getAppFilename()
envVarsPrefix = appInvocation()
): Configuration {.raises: [ConfigurationError].} =
## Loads a program configuration by parsing command-line arguments
## and a standard set of config files that can specify:
Expand All @@ -928,7 +931,6 @@ proc loadImpl[C, SecondarySources](

# This is an initial naive implementation that will be improved
# over time.

let (rootCmd, fieldSetters) = configurationRtti(Configuration)
var fieldCounters: array[fieldSetters.len, int]

Expand All @@ -941,11 +943,11 @@ proc loadImpl[C, SecondarySources](
var help = ""

proc suggestCallingHelp =
errorOutput "Try ", fgCommand, ("$1 --help" % appInvocation())
errorOutput "Try ", fgCommand, appInvocation() & " --help"
errorOutput " for more information.\p"
flushOutputAndQuit QuitFailure

template fail(args: varargs[untyped]) =
template fail(args: varargs[untyped]): untyped =
if quitOnFailure:
errorOutput args
errorOutput "\p"
Expand All @@ -954,14 +956,13 @@ proc loadImpl[C, SecondarySources](
# TODO: populate this string
raise newException(ConfigurationError, "")

let confAddr = addr result

template applySetter(setterIdx: int, cmdLineVal: string) =
template applySetter(
conf: Configuration, setterIdx: int, cmdLineVal: string): untyped =
when defined(nimHasWarnBareExcept):
{.push warning[BareExcept]:off.}

try:
fieldSetters[setterIdx][1](confAddr[], some(cmdLineVal))
fieldSetters[setterIdx][1](conf, some(cmdLineVal))
inc fieldCounters[setterIdx]
except:
fail("Error while processing the ",
Expand All @@ -979,10 +980,11 @@ proc loadImpl[C, SecondarySources](
template required(opt: OptInfo): bool =
fieldSetters[opt.idx][3] and not opt.hasDefault

template activateCmd(discriminator: OptInfo, activatedCmd: CmdInfo) =
template activateCmd(
conf: Configuration, discriminator: OptInfo, activatedCmd: CmdInfo) =
let cmd = activatedCmd
applySetter(discriminator.idx, if cmd.desc.len > 0: cmd.desc
else: cmd.name)
conf.applySetter(discriminator.idx, if cmd.desc.len > 0: cmd.desc
else: cmd.name)
activeCmds.add cmd
nextArgIdx = cmd.getNextArgIdx(-1)

Expand Down Expand Up @@ -1117,14 +1119,14 @@ proc loadImpl[C, SecondarySources](
let defaultCmd = subCmdDiscriminator.subCmds[subCmdDiscriminator.defaultSubCmd]
opt = findOpt(defaultCmd.opts, key)
if opt != nil:
activateCmd(subCmdDiscriminator, defaultCmd)
result.activateCmd(subCmdDiscriminator, defaultCmd)
else:
discard

if opt != nil:
applySetter(opt.idx, val)
result.applySetter(opt.idx, val)
else:
fail "Unrecognized option '$1'" % [key]
fail "Unrecognized option '" & key & "'"

of cmdArgument:
if lastCmd.hasSubCommands:
Expand All @@ -1135,13 +1137,13 @@ proc loadImpl[C, SecondarySources](
if subCmdDiscriminator != nil:
let subCmd = findCmd(subCmdDiscriminator.subCmds, key)
if subCmd != nil:
activateCmd(subCmdDiscriminator, subCmd)
result.activateCmd(subCmdDiscriminator, subCmd)
break processArg

if nextArgIdx == -1:
fail lastCmd.noMoreArgsError

applySetter(nextArgIdx, key)
result.applySetter(nextArgIdx, key)

if not fieldSetters[nextArgIdx][4]:
nextArgIdx = lastCmd.getNextArgIdx(nextArgIdx)
Expand All @@ -1154,13 +1156,13 @@ proc loadImpl[C, SecondarySources](
subCmdDiscriminator.defaultSubCmd != -1 and
fieldCounters[subCmdDiscriminator.idx] == 0:
let defaultCmd = subCmdDiscriminator.subCmds[subCmdDiscriminator.defaultSubCmd]
activateCmd(subCmdDiscriminator, defaultCmd)
result.activateCmd(subCmdDiscriminator, defaultCmd)

if secondarySources != nil:
if not isNil(secondarySources): # Nim v2.0.10: `!= nil` broken in nimscript
Copy link
Contributor

Choose a reason for hiding this comment

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

type SecondarySources = object

proc loadImpl[C, SecondarySources](
    Configuration: typedesc[C],
    secondarySourcesRef: ref SecondarySources,
    secondarySources: proc (
        config: Configuration, sources: ref SecondarySources
    ) {.gcsafe.} = nil): Configuration =
  if secondarySources != nil:
    secondarySources(result, secondarySourcesRef)

template load(
    Configuration: type,
    secondarySources: untyped = nil): untyped =
  block:
    let secondarySourcesRef = new SecondarySources
    loadImpl(Configuration, secondarySourcesRef, secondarySources)

type
  CliConfig_33558379 = object
let config_33558380 = load(CliConfig_33558379)

is repro, and it's an issue in v2.0.10 and version-2-0 but not version-2-2 or devel. Using isNil seems fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it worked for me in nim c mode and only bugs out in nim e nimscript mode.
do you have a link that I should refer to in the comment here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't, because it seems to already be fixed in Nim upstream, and it's not really worth a backport if it's just allowing != nil instead of not isNil.

Can link to this comment in GitHub perhaps.

try:
secondarySources(result, secondarySourcesRef)
except ConfigurationError as err:
fail "Failed to load secondary sources: '$1'" % [err.msg]
fail "Failed to load secondary sources: '" & err.msg & "'"

proc processMissingOpts(
conf: var Configuration, cmd: CmdInfo) {.raises: [ConfigurationError].} =
Expand All @@ -1171,17 +1173,17 @@ proc loadImpl[C, SecondarySources](
try:
if existsEnv(envKey):
let envContent = getEnv(envKey)
applySetter(opt.idx, envContent)
conf.applySetter(opt.idx, envContent)
elif secondarySourcesRef.setters[opt.idx](conf, secondarySourcesRef):
# all work is done in the config file setter,
# there is nothing left to do here.
discard
elif opt.hasDefault:
fieldSetters[opt.idx][1](conf, none[string]())
elif opt.required:
fail "The required option '$1' was not specified" % [opt.name]
fail "The required option '" & opt.name & "' was not specified"
except ValueError as err:
fail "Option '$1' failed to parse: '$2'" % [opt.name, err.msg]
fail "Option '" & opt.name & "' failed to parse: '" & err.msg & "'"

for cmd in activeCmds:
result.processMissingOpts(cmd)
Expand All @@ -1194,10 +1196,9 @@ template load*(
printUsage = true,
quitOnFailure = true,
secondarySources: untyped = nil,
envVarsPrefix = getAppFilename()): untyped =

envVarsPrefix = appInvocation()): untyped =
block:
var secondarySourcesRef = generateSecondarySources(Configuration)
let secondarySourcesRef = generateSecondarySources(Configuration)
loadImpl(Configuration, cmdLine, version,
copyrightBanner, printUsage, quitOnFailure,
secondarySourcesRef, secondarySources, envVarsPrefix)
Expand Down
18 changes: 18 additions & 0 deletions confutils.nimble
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,21 @@ task test, "Run all tests":
else:
echo " [FAILED] ", path.split(DirSep)[^1]
quit(QuitFailure)

echo "\r\nNimscript test:"
let
actualOutput = gorgeEx(
nimc & " --verbosity:0 e " & flags & " " & "./tests/cli_example.nim " &
"--foo=1 --bar=2 --withBaz 42").output
expectedOutput = unindent"""
foo = 1
bar = 2
baz = true
arg ./tests/cli_example.nim
arg 42"""
if expectedOutput == actualOutput:
echo " [OK] tests/cli_example.nim"
else:
echo " [FAILED] tests/cli_example.nim"
echo actualOutput
quit(QuitFailure)
Loading