sync: improve error messages for invalid update options

src/cli.nim

@@ -295,10 +295,17 @@ func genHelpText: string =
       result.add alignLeft(syntax[opt], maxLen) & optionDescriptions[opt] & "\n"
   setLen(result, result.len - 1)
 
-proc showHelp(exitCode: range[0..255] = 0) =
-  const helpText = genHelpText().compress()
+func firstLine(s: string): string =
+  s[0 ..< s.find('\n')]
+
+proc showHelp(exitCode: range[0..255] = 0, prependDashedLine = false) =
+  const helpCompressed = genHelpText().compress()
+  let helpUncompressed = helpCompressed.uncompress()
   let f = if exitCode == 0: stdout else: stderr
-  f.writeLine helpText.uncompress()
+  if prependDashedLine:
+    let dashLen = helpUncompressed.firstLine().len
+    f.write(&"\n\n{'-'.repeat(dashLen)}\n\n")
+  f.writeLine helpUncompressed
   if f == stdout:
     f.flushFile()
   quit(exitCode)
@@ -323,8 +330,7 @@ proc showError*(s: string) =
   else:
     stderr.write(errorPrefix)
   stderr.write(s)
-  stderr.write("\n\n")
-  showHelp(exitCode = 1)
+  showHelp(exitCode = 1, prependDashedLine = true)
 
 func formatOpt(kind: CmdLineKind, key: string, val = ""): string =
   ## Returns a string that describes an option, given its `kind`, `key` and

src/sync/sync.nim

@@ -8,26 +8,36 @@ proc validate(conf: Conf) =
   ## combination of options.
   if conf.action.update:
     if conf.action.yes and skTests in conf.action.scope and conf.action.tests == tmChoose:
-      let msg = fmt"""
-        '{list(optFmtSyncYes)}' was provided to non-interactively update, but the tests updating mode is still 'choose'.
+      const msg = """
+        '-y, --yes' was provided to non-interactively update, but tests are in
+        the syncing scope and the tests updating mode is 'choose'.
+
         You can either:
-        - remove '{list(optFmtSyncYes)}', and update by confirming prompts
-        - or narrow the syncing scope via some combination of --docs, --filepaths, and --metadata
-        - or add '--tests include' or '--tests exclude' to non-interactively include/exclude missing tests
+        - use '--tests include' or '--tests exclude' to non-interactively include/exclude
+          missing tests
+        - or narrow the syncing scope via some combination of '--docs', '--filepaths', and
+          '--metadata' (removing '--tests' if it was passed)
+        - or remove '-y, --yes', and update by answering prompts
+
         If no syncing scope option is provided, configlet uses the full syncing scope.
-        If no --tests value is provided, configlet uses the 'choose' mode.""".unindent()
+        If '--tests' is provided without an argument, configlet uses the 'choose' mode.""".dedent(8)
       showError(msg)
-    if not isatty(stdin):
-      if not conf.action.yes:
-        let intersection = conf.action.scope * {skDocs, skFilepaths, skMetadata}
-        if intersection.len > 0:
-          let msg = fmt"""
-            Configlet was used in a non-interactive context, and the --update option was passed without the --yes option
-            You can either:
-            - keep using configlet non-interactively, and remove the --update option so that no destructive changes are performed
-            - keep using configlet non-interactively, and add the --yes option to perform destructive changes
-            - use configlet in an interactive terminal""".unindent()
-          showError(msg)
+    if not conf.action.yes and not isatty(stdin):
+      let intersection = conf.action.scope * {skDocs, skFilepaths, skMetadata}
+      if intersection.len > 0:
+        const msg = """
+          configlet ran in a non-interactive context, but interaction was required because
+          '--update' was passed without '--yes', and at least one of docs, filepaths, and
+          metadata were in the syncing scope.
+
+          You can either:
+          - keep using configlet non-interactively, and add the '--yes' option to perform
+            changes without confirmation
+          - or keep using configlet non-interactively, and remove the '--update' option so
+            that configlet performs no changes
+          - or run the same command in an interactive terminal, to update by answering
+            prompts""".dedent(10)
+        showError(msg)
 
 type
   TrackExerciseSlugs* = object

tests/test_binary.nim

@@ -520,6 +520,57 @@ proc testsForSync(binaryPath: static string) =
     testDiffThenRestore(trackDir, expectedDiff, configPaths)
 
   suite "sync, with --update and --tests":
+    const expectedErrorUpdateChoose = """
+      '-y, --yes' was provided to non-interactively update, but tests are in
+      the syncing scope and the tests updating mode is 'choose'.
+
+      You can either:
+      - use '--tests include' or '--tests exclude' to non-interactively include/exclude
+        missing tests
+      - or narrow the syncing scope via some combination of '--docs', '--filepaths', and
+        '--metadata' (removing '--tests' if it was passed)
+      - or remove '-y, --yes', and update by answering prompts
+
+      If no syncing scope option is provided, configlet uses the full syncing scope.
+      If '--tests' is provided without an argument, configlet uses the 'choose' mode.
+    """.dedent(6)
+
+    for options in ["-y", "-y --docs", "-y --filepaths", "-y --metadata",
+                    "choose -y", "choose -y --docs", "choose -y --filepaths",
+                    "choose -y --metadata"]:
+      test &"--tests {options}: prints an error message, and exits with 1":
+        let (outp, exitCode) = execCmdEx(&"{syncOfflineUpdateTests} {options}")
+        check:
+          outp.contains(expectedErrorUpdateChoose)
+          exitCode == 1
+        checkNoDiff(trackDir)
+
+    const expectedErrorUpdateChooseNonInteractive = """
+      configlet ran in a non-interactive context, but interaction was required because
+      '--update' was passed without '--yes', and at least one of docs, filepaths, and
+      metadata were in the syncing scope.
+
+      You can either:
+      - keep using configlet non-interactively, and add the '--yes' option to perform
+        changes without confirmation
+      - or keep using configlet non-interactively, and remove the '--update' option so
+        that configlet performs no changes
+      - or run the same command in an interactive terminal, to update by answering
+        prompts
+    """.dedent(6)
+
+    for options in ["", "--docs", "--filepaths", "--metadata", "--tests --docs",
+                    "--tests --filepaths", "--tests --metadata",
+                    "--docs --filepaths", "--docs --metadata",
+                    "--docs --filepaths --metadata",
+                    "--docs --filepaths --metadata --tests"]:
+      test &"{options}: prints an error message, and exits with 1":
+        let (outp, exitCode) = execCmdEx(&"{syncOfflineUpdate} {options}")
+        check:
+          outp.contains(expectedErrorUpdateChooseNonInteractive)
+          exitCode == 1
+        checkNoDiff(trackDir)
+
     const
       expectedOutputAnagramInclude = fmt"""
         {header}
@@ -590,6 +641,14 @@ proc testsForSync(binaryPath: static string) =
       execAndCheck(0, &"{syncOfflineUpdateTests} exclude -e anagram", expectedOutputAnagramExclude)
     testDiffThenRestore(trackDir, expectedAnagramDiffExclude, anagramTestsTomlPath)
 
+    test &"--tests include -y: includes a missing test case for a given exercise, and exits with 0":
+      execAndCheck(0, &"{syncOfflineUpdateTests} include -e anagram -y", expectedOutputAnagramInclude)
+    testDiffThenRestore(trackDir, expectedAnagramDiffInclude, anagramTestsTomlPath)
+
+    test &"--tests exclude -y: excludes a missing test case for a given exercise, and exits with 0":
+      execAndCheck(0, &"{syncOfflineUpdateTests} exclude -e anagram -y", expectedOutputAnagramExclude)
+    testDiffThenRestore(trackDir, expectedAnagramDiffExclude, anagramTestsTomlPath)
+
     test "--tests choose: includes a missing test case for a given exercise when the input is 'y', and exits with 0":
       execAndCheckExitCode(0, &"{syncOfflineUpdateTests} choose -e anagram",
                            inputStr = "y")