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

Feature: Add --trackDir option #72

Merged
merged 3 commits into from
Jan 26, 2021
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Options for uuid:
Global options:
-h, --help Show this help message and exit
--version Show this tool's version information and exit
-t, --track-dir <dir> Specify a track directory to use instead of the current directory
-v, --verbosity <verbosity> The verbosity of output. Allowed values: q[uiet], n[ormal], d[etailed]
```

Expand Down
12 changes: 10 additions & 2 deletions src/cli.nim
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ type

Conf* = object
action*: Action
trackDir*: string
verbosity*: Verbosity

Opt = enum
optHelp = "help"
optVersion = "version"
optTrackDir = "trackDir"
optVerbosity = "verbosity"
optSyncExercise = "exercise"
optSyncCheck = "check"
Expand Down Expand Up @@ -110,6 +112,7 @@ func genHelpText: string =
for opt in Opt:
let paramName =
case opt
of optTrackDir: "dir"
of optVerbosity: "verbosity"
of optSyncExercise: "slug"
of optSyncMode: "mode"
Expand All @@ -127,6 +130,7 @@ func genHelpText: string =
const descriptions: array[Opt, string] = [
optHelp: "Show this help message and exit",
optVersion: "Show this tool's version information and exit",
optTrackDir: "Specify a track directory to use instead of the current directory",
optVerbosity: &"The verbosity of output. {allowedValues(Verbosity)}",
optSyncExercise: "Only sync this exercise",
optSyncCheck: "Terminates with a non-zero exit code if one or more tests " &
Expand Down Expand Up @@ -213,9 +217,11 @@ func initAction*(actionKind: ActionKind, probSpecsDir = ""): Action =
of actUuid:
Action(kind: actionKind, num: 1)

func initConf*(action = initAction(actNil), verbosity = verNormal): Conf =
func initConf*(action = initAction(actNil), trackDir = getCurrentDir(),
verbosity = verNormal): Conf =
result = Conf(
action: action,
trackDir: trackDir,
verbosity: verbosity,
)

Expand Down Expand Up @@ -285,7 +291,7 @@ proc handleArgument(conf: var Conf; kind: CmdLineKind; key: string) =
if conf.action.kind == actNil:
let actionKind = parseActionKind(key)
let action = initAction(actionKind)
conf = initConf(action, conf.verbosity)
conf = initConf(action, conf.trackDir, conf.verbosity)
else:
showError(&"invalid argument for command '{conf.action.kind}': '{key}'")

Expand All @@ -310,6 +316,8 @@ proc handleOption(conf: var Conf; kind: CmdLineKind; key, val: string) =
showHelp()
of optVersion:
showVersion()
of optTrackDir:
setGlobalOpt(trackDir, val)
of optVerbosity:
setGlobalOpt(verbosity, parseVal[Verbosity](kind, key, val))
else:
Expand Down
8 changes: 4 additions & 4 deletions src/sync/exercises.nim
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ proc status*(exercise: Exercise): ExerciseStatus =
proc hasCanonicalData*(exercise: Exercise): bool =
exercise.testCases.len > 0

proc testsFile(exercise: Exercise): string =
getCurrentDir() / "exercises" / exercise.slug / ".meta" / "tests.toml"
proc testsFile(exercise: Exercise, trackDir: string): string =
trackDir / "exercises" / exercise.slug / ".meta" / "tests.toml"

proc toToml(exercise: Exercise): string =
result.add("[canonical-tests]\n")
Expand All @@ -89,8 +89,8 @@ proc toToml(exercise: Exercise): string =
result.add(&"\n# {testCase.description}")
result.add(&"\n\"{testCase.uuid}\" = {isIncluded}\n")

proc writeTestsToml*(exercise: Exercise) =
let testsPath = testsFile(exercise)
proc writeTestsToml*(exercise: Exercise, trackDir: string) =
let testsPath = testsFile(exercise, trackDir)
createDir(testsPath.parentDir())

let contents = toToml(exercise)
Expand Down
11 changes: 6 additions & 5 deletions src/sync/sync.nim
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ proc syncDecision(testCase: ExerciseTestCase, mode: Mode): SyncDecision =
of modeChoose:
chooseSyncDecision(testCase)

proc sync(exercise: Exercise, mode: Mode): Exercise =
proc sync(exercise: Exercise, conf: Conf): Exercise =
result = exercise

let mode = conf.action.mode
case mode
of modeInclude:
logNormal(&"[info] {exercise.slug}: included {exercise.tests.missing.len} missing test cases")
Expand Down Expand Up @@ -100,13 +101,13 @@ proc sync(exercise: Exercise, mode: Mode): Exercise =

result.tests = initExerciseTests(included, excluded, missing)

writeTestsToml(result)
writeTestsToml(result, conf.trackDir)
Copy link
Member

Choose a reason for hiding this comment

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

Is conf.trackDir set to the current directory if the --track-dir argument is not specified?

Copy link
Member Author

@ee7 ee7 Jan 26, 2021

Choose a reason for hiding this comment

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

It was set to the empty string, which ends up meaning the current directory.

From the examples in https://nim-lang.github.io/Nim/os.html#/,string,string

`assert "" / "lib" == "lib"`

So

  trackDir / "exercises" / exercise.slug / ".meta" / "tests.toml"

was the same as

  "exercises" / exercise.slug / ".meta" / "tests.toml"

when trackDir is empty.

I pushed a new commit, do you prefer it?


proc sync(exercises: seq[Exercise], mode: Mode): seq[Exercise] =
proc sync(exercises: seq[Exercise], conf: Conf): seq[Exercise] =
for exercise in exercises:
case exercise.status
of exOutOfSync:
result.add(sync(exercise, mode))
result.add(sync(exercise, conf))
of exInSync:
logDetailed(&"[skip] {exercise.slug} is up-to-date")
of exNoCanonicalData:
Expand All @@ -116,7 +117,7 @@ proc sync*(conf: Conf) =
logNormal("Syncing exercises...")

let exercises = findExercises(conf)
let syncedExercises = sync(exercises, conf.action.mode)
let syncedExercises = sync(exercises, conf)

if syncedExercises.anyIt(it.status == exOutOfSync):
logNormal("[warn] some exercises are still missing test cases")
Expand Down
5 changes: 1 addition & 4 deletions src/sync/tracks.nim
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ type
tests*: TrackExerciseTests
repoExercise: TrackRepoExercise

proc newTrackRepo: TrackRepo =
result.dir = getCurrentDir()

Comment on lines -27 to -29
Copy link
Member Author

@ee7 ee7 Jan 26, 2021

Choose a reason for hiding this comment

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

I made this change partly because I find it more readable to inline it in findTrackExercises.

But there's also a general issue that I'll try to fix everywhere eventually: I believe that procs that initialize non-ref objects should be named starting with init, not new - even if they contain a field of a type like string.

From the style guide: https://nim-lang.github.io/Nim/nep1.html#introduction-naming-conventions

The stdlib API is designed to be easy to use and consistent. Ease of use is measured by the number of calls to achieve a concrete high level action. The ultimate goal is that the programmer can guess a name.

The library uses a simple naming scheme that makes use of common abbreviations to keep the names short but meaningful.

English word To use Notes
initialize initT init is used to create a value type T
new newP new is used to create a reference type P
... ... ...

For example, in the stdlib sets module, we write initHashSet for an object named HashSet, even though that object has a field of type seq

But in my refactoring branch I think I changed a lot of this stuff to use distinct string rather than an one-field object anyway.

proc configJsonFile(repo: TrackRepo): string =
repo.dir / "config.json"

Expand Down Expand Up @@ -83,5 +80,5 @@ proc findTrackExercises(repo: TrackRepo, conf: Conf): seq[TrackExercise] =
result.add(newTrackExercise(repoExercise))

proc findTrackExercises*(conf: Conf): seq[TrackExercise] =
let trackRepo = newTrackRepo()
let trackRepo = TrackRepo(dir: conf.trackDir)
trackRepo.findTrackExercises(conf)