From 2b70195c499d294a0d939e48ea54d084fac1695c Mon Sep 17 00:00:00 2001 From: ee7 <45465154+ee7@users.noreply.github.com> Date: Sat, 8 May 2021 17:46:47 +0200 Subject: [PATCH] sync(tracks): refactor (#307) This commit is part of a series that tries to improve readability and maintainability of the sync code. I've limited the scope of this commit mostly to the non-exported functionality. Changes include: - Prefer `distinct string` to an `object` with one `string` field. - Inline some trivial procs that are only called in one place. - Improve some identifier names. In particular, try to better distinguish between file paths and slugs. - Remove an unnecessary field. - Add some `requiresInit` annotations. - General style tweaks. This is a squashed commit. See #307 for the pre-squash commits. See also: - #64, which tracks the refactoring of the sync code. --- src/sync/tracks.nim | 125 +++++++++++++++++++----------------------- tests/test_tracks.nim | 7 +-- 2 files changed, 60 insertions(+), 72 deletions(-) diff --git a/src/sync/tracks.nim b/src/sync/tracks.nim index 9516243c..1dabb3a6 100644 --- a/src/sync/tracks.nim +++ b/src/sync/tracks.nim @@ -3,17 +3,9 @@ import pkg/parsetoml import ".."/cli type - ConfigJsonExercise = object - slug: string + TrackDir {.requiresInit.} = distinct string - ConfigJson = object - practice: seq[ConfigJsonExercise] - - TrackRepo = object - dir: string - - TrackRepoExercise = object - dir: string + PracticeExercisePath {.requiresInit.} = distinct string TrackExerciseTests* = object included*: HashSet[string] @@ -22,73 +14,70 @@ type TrackExercise* = object slug*: string tests*: TrackExerciseTests - repoExercise: TrackRepoExercise - -func configJsonFile(repo: TrackRepo): string = - repo.dir / "config.json" - -func exercisesDir(repo: TrackRepo): string = - repo.dir / "exercises" -func practiceExerciseDir(repo: TrackRepo, exercise: ConfigJsonExercise): string = - repo.exercisesDir / "practice" / exercise.slug +proc `/`(head: TrackDir, tail: string): string {.borrow.} +proc `/`(head: PracticeExercisePath, tail: string): string {.borrow.} +proc extractFilename(exercisePath: PracticeExercisePath): string {.borrow.} -func slug(exercise: TrackRepoExercise): string = - extractFilename(exercise.dir) +func slug(exercisePath: PracticeExercisePath): string = + extractFilename(exercisePath) -func testsFile(exercise: TrackRepoExercise): string = - exercise.dir / ".meta" / "tests.toml" +func testsFile(exercisePath: PracticeExercisePath): string = + exercisePath / ".meta" / "tests.toml" func testsFile*(exercise: TrackExercise): string = - exercise.repoExercise.testsFile - -proc parseConfigJson(filePath: string): ConfigJson = - let json = json.parseFile(filePath)["exercises"] - to(json, ConfigJson) - -func initTrackRepoExercise(repo: TrackRepo, - exercise: ConfigJsonExercise): TrackRepoExercise = - TrackRepoExercise(dir: repo.practiceExerciseDir(exercise)) - -proc exercises(repo: TrackRepo): seq[TrackRepoExercise] = - let config = parseConfigJson(repo.configJsonFile) - - for exercise in config.practice: - result.add(initTrackRepoExercise(repo, exercise)) - -proc initTrackExerciseTests(exercise: TrackRepoExercise): TrackExerciseTests = - if not fileExists(exercise.testsFile): - return - - let tests = parsetoml.parseFile(exercise.testsFile) - - for uuid, val in tests.getTable(): - if val.hasKey("include"): - if val["include"].kind == Bool: - let isIncluded = val["include"].getBool() - if isIncluded: - result.included.incl(uuid) + PracticeExercisePath("").testsFile() + +proc getPracticeExercisePaths(trackDir: TrackDir): seq[PracticeExercisePath] = + let config = json.parseFile(trackDir / "config.json")["exercises"] + + if config.hasKey("practice"): + let practiceExercises = config["practice"] + result = newSeqOfCap[PracticeExercisePath](practiceExercises.len) + + for exercise in practiceExercises: + if exercise.hasKey("slug"): + if exercise["slug"].kind == JString: + let slug = exercise["slug"].getStr() + let path = trackDir / "exercises" / "practice" / slug + result.add PracticeExercisePath(path) + +proc initTrackExerciseTests(exercisePath: PracticeExercisePath): TrackExerciseTests = + let testsFile = testsFile(exercisePath) + if fileExists(testsFile): + let tests = parsetoml.parseFile(testsFile) + + for uuid, val in tests.getTable(): + if val.hasKey("include"): + if val["include"].kind == Bool: + let isIncluded = val["include"].getBool() + if isIncluded: + result.included.incl(uuid) + else: + result.excluded.incl(uuid) else: - result.excluded.incl(uuid) + let msg = "Error: the value of an `include` key is `" & + val["include"].toTomlString() & "`, but it must be a " & + "bool:\n" & exercisePath.testsFile() + stderr.writeLine(msg) else: - let msg = "Error: the value of an `include` key is `" & - val["include"].toTomlString() & "`, but it must be a bool:\n" & - exercise.testsFile() - stderr.writeLine(msg) - else: - result.included.incl(uuid) - -proc initTrackExercise(exercise: TrackRepoExercise): TrackExercise = + result.included.incl(uuid) + +proc initTrackExercise(exercisePath: PracticeExercisePath): TrackExercise = TrackExercise( - slug: exercise.slug, - tests: initTrackExerciseTests(exercise), + slug: slug(exercisePath), + tests: initTrackExerciseTests(exercisePath), ) -proc findTrackExercises(repo: TrackRepo, conf: Conf): seq[TrackExercise] = - for repoExercise in repo.exercises: - if conf.action.exercise.len == 0 or conf.action.exercise == repoExercise.slug: - result.add(initTrackExercise(repoExercise)) +proc findTrackExercises(trackDir: TrackDir, + userExercise: string): seq[TrackExercise] = + let practiceExercisePaths = getPracticeExercisePaths(trackDir) + result = newSeqOfCap[TrackExercise](practiceExercisePaths.len) + + for practiceExercisePath in practiceExercisePaths: + if userExercise.len == 0 or userExercise == slug(practiceExercisePath): + result.add initTrackExercise(practiceExercisePath) proc findTrackExercises*(conf: Conf): seq[TrackExercise] = - let trackRepo = TrackRepo(dir: conf.trackDir) - trackRepo.findTrackExercises(conf) + let trackDir = TrackDir(conf.trackDir) + result = findTrackExercises(trackDir, conf.action.exercise) diff --git a/tests/test_tracks.nim b/tests/test_tracks.nim index d6be86af..071443cb 100644 --- a/tests/test_tracks.nim +++ b/tests/test_tracks.nim @@ -50,8 +50,7 @@ proc main = const expectedHelloWorld = """ (slug: "hello-world", tests: (included: {"af9ffe10-dc13-42d8-a742-e7bdafac449d"}, - excluded: {}), - repoExercise: (dir: "")) + excluded: {})) """.oneLine() check: @@ -63,9 +62,9 @@ proc main = tests: (included: {"1cf3e15a-a3d7-4a87-aeb3-ba1b43bc8dce", "3549048d-1a6e-4653-9a79-b0bda163e8d5", "b4c6dbb8-b4fb-42c2-bafd-10785abe7709"}, - excluded: {}), - repoExercise: (dir: "")) + excluded: {})) """.oneLine() + check: trackExercises[1].`$` == expectedTwoFer