completions: add completions for bash and fish #629

glennj · 2022-07-08T23:47:05Z

Not being a zsh or PS user, those completions are not included.

Changes to fetch-configlet to detect user's shell and print instructions will follow in a separate PR.

Closes #615

ErikSchierboom · 2022-07-12T07:56:03Z

Thanks! I'll let ee7 review this, as he is much more well-versed into this than I am.

Update create-artifact script to include completions in artifact.

ee7

Thanks for working on this. Very cool.

First pass at a review. Great that it's even got slug completion, but does it find the slugs at "tab time" or at source time? The latter would be a problem when working on multiple tracks. (I haven't tested yet, sorry).

~~I see that db12f3b..10dd1e0 removed the shell detection - are you leaning towards not trying to detect it?~~

Edit: Oh, from #615 (comment) I guess you'll make a separate PR for the fetch-configlet changes.

ee7 · 2022-07-12T12:40:02Z

scripts/fetch-configlet

+emit_completion_command() {
+    local shell file
+    if command -v getent >/dev/null; then
+        shell=$(getent passwd "${USER}" | cut -d: -f6)


Suggested change

shell=$(getent passwd "${USER}" | cut -d: -f6)

shell=$(getent passwd "${USER}" | cut -d: -f7)

The format for /etc/passwd is the below, right?

account:password:UID:GID:GECOS:directory:shell

So we need the seventh field to get the shell?

For example, on Arch Linux:

$ getent passwd "${USER}" myusername:x:1000:1000::/home/myusername:/bin/zsh $ getent passwd "${USER}" | cut -d: -f6 /home/myusername $ getent passwd "${USER}" | cut -d: -f7 /bin/zsh

On OpenBSD:

$ getent passwd "${USER}" myusername:*:1000:1000:myusername:/home/myusername:/bin/ksh

Good catch. Will fix in next PR.

ee7 · 2022-07-12T12:40:27Z

scripts/fetch-configlet

+    elif [[ -f /etc/passwd ]]; then
+        shell=$(awk -F: -v u="${USER}" '$1 == u {print $6}' /etc/passwd)


Suggested change

elif [[ -f /etc/passwd ]]; then

shell=$(awk -F: -v u="${USER}" '$1 == u {print $6}' /etc/passwd)

elif [[ -f /etc/passwd ]]; then

shell=$(awk -F: -v u="${USER}" '$1 == u {print $7}' /etc/passwd)

Ditto.

ee7 · 2022-07-12T12:53:32Z

completions/configlet.bash

+_configlet_complete_generate_() {
+    _configlet_complete_options_ "$global_opts"
+}


For later: configlet generate will (soon-ish) have the same non-destructive-by-default behavior as configlet fmt, and so support the same options (-u, -y, -e) . So we should remember to update the completions accordingly.

When that happens, we can just dispatch:

_configlet_complete_generate_() { _configlet_complete_fmt_ }

completions/configlet.bash

completions/configlet.fish

glennj · 2022-07-12T14:05:42Z

does it find the slugs at "tab time" or at source time?

At tab time.

Instead, it looks like we'll bake the completions into the configlet binary via a new subcommand [1]. [1] exercism#615 (comment)

We tend to use "prob-specs" rather than "prob-spec" to abbreviate "problem-specifications". Furthermore, configlet used to have a --prob-specs-dir option (not --prob-spec-dir), and configlet still prints an advisory message when a user writes --prob-specs-dir. $ git grep --heading --break --ignore-case 'prob.*spec[^i]' src/cli.nim 394: if keyNormalized in ["p", "probspecsdir"]: 396: The --prob-specs-dir option was removed in configlet 4.0.0-beta.1 (April 2022). src/info/info.nim 3:import ".."/[cli, sync/probspecs, types_track_config] 58: ProbSpecsExercises = object 63:proc init(T: typedesc[ProbSpecsExercises], probSpecsDir: ProbSpecsDir): T = 65: for kind, path in walkDir(probSpecsDir / "exercises"): 75:proc unimplementedProbSpecsExercises(practiceExercises: seq[PracticeExercise], 77: probSpecsDir: ProbSpecsDir): string = 79: probSpecsExercises = ProbSpecsExercises.init(probSpecsDir) 83: uWith = probSpecsExercises.withCanonicalData - practiceExerciseSlugs - foregone 84: uWithout = probSpecsExercises.withoutCanonicalData - practiceExerciseSlugs - foregone 133: let probSpecsDir = ProbSpecsDir.init(conf) 135: echo unimplementedProbSpecsExercises(t.exercises.practice, t.exercises.foregone, 136: probSpecsDir) src/sync/exercises.nim 4:import "."/[probspecs, tracks] 5:export tracks.`$`, probspecs.pretty 11: json*: ProbSpecsTestCase 38: probSpecsTestCases: ProbSpecsTestCases): T = 40: for testCase in probSpecsTestCases: 49:proc new(T: typedesc[ExerciseTestCase], testCase: ProbSpecsTestCase): T = 56:proc getReimplementations(testCases: ProbSpecsTestCases): Table[string, string] = 65:proc init(T: typedesc[ExerciseTestCases], testCases: ProbSpecsTestCases): T = 84:iterator findExercises*(conf: Conf, probSpecsDir: ProbSpecsDir): Exercise {.inline.} = 88: let testCases = getCanonicalTests(probSpecsDir, practiceExercise.slug.string) src/sync/probspecs.nim 5: ProbSpecsDir* {.requiresInit.} = distinct string 7: ProbSpecsExerciseDir {.requiresInit.} = distinct string 9: ProbSpecsTestCase* = distinct JsonNode 11: ProbSpecsTestCases* = seq[ProbSpecsTestCase] 13:proc `$`(dir: ProbSpecsDir): string {.borrow.} 14:proc dirExists(dir: ProbSpecsDir): bool {.borrow.} 15:proc createDir(dir: ProbSpecsDir) {.borrow.} 16:proc parentDir(path: ProbSpecsDir): string {.borrow.} 17:proc `/`*(head: ProbSpecsDir, tail: string): string {.borrow.} 18:proc `/`(head: ProbSpecsExerciseDir, tail: string): string {.borrow.} 19:proc lastPathPart(path: ProbSpecsExerciseDir): string {.borrow.} 20:proc `[]`(testCase: ProbSpecsTestCase, name: string): JsonNode {.borrow.} 21:proc hasKey(testCase: ProbSpecsTestCase, key: string): bool {.borrow.} 22:proc pretty*(testCase: ProbSpecsTestCase, indent = 2): string {.borrow.} 24:func canonicalDataFile(probSpecsExerciseDir: ProbSpecsExerciseDir): string = 25: probSpecsExerciseDir / "canonical-data.json" 27:func slug(probSpecsExerciseDir: ProbSpecsExerciseDir): string = 28: lastPathPart(probSpecsExerciseDir) 30:proc uuid*(testCase: ProbSpecsTestCase): string = 33:proc description*(testCase: ProbSpecsTestCase): string = 36:func isReimplementation*(testCase: ProbSpecsTestCase): bool = 39:proc reimplements*(testCase: ProbSpecsTestCase): string = 42:proc init(T: typedesc[ProbSpecsTestCases], node: JsonNode, prefix = ""): T = 50: result.add ProbSpecsTestCase(node) 58: result.add ProbSpecsTestCases.init(childNode, prefix) 69:proc parseProbSpecsTestCases(probSpecsExerciseDir: ProbSpecsExerciseDir): ProbSpecsTestCases = 72: let canonicalJsonPath = canonicalDataFile(probSpecsExerciseDir) 74: if slug(probSpecsExerciseDir) == "grains": 78: result = ProbSpecsTestCases.init(j) 80:proc getCanonicalTests*(probSpecsDir: ProbSpecsDir, 81: slug: string): ProbSpecsTestCases = 83: ## `probSpecsDir`. 84: let probSpecsExerciseDir = joinPath(probSpecsDir.string, "exercises", 85: slug).ProbSpecsExerciseDir 86: if fileExists(probSpecsExerciseDir.canonicalDataFile()): 87: result = parseProbSpecsTestCases(probSpecsExerciseDir) 89:proc getNameOfRemote*(probSpecsDir: ProbSpecsDir; 91: ## Returns the name of the remote in `probSpecsDir` that points to `location` 97: &"problem-specifications directory: '{probSpecsDir}'" 98: let remotes = gitCheck(0, ["-C", probSpecsDir.string, "remote", "-v"], msg) 105: &"the cached problem-specifications directory: '{probSpecsDir}'") 111:proc validate(probSpecsDir: ProbSpecsDir, conf: Conf) = 112: ## Raises an error if the given `probSpecsDir` is not a valid 116: logDetailed(&"Using cached 'problem-specifications' dir: {probSpecsDir}") 118: withDir probSpecsDir.string: 124: &"git repository: '{probSpecsDir}'") 128: &"has an unexpected initial commit: '{probSpecsDir}'") 133: &"'{probSpecsDir}'") 139: # prob-specs repo. 144: &"branch: '{probSpecsDir}'") 147: # prob-specs state that hasn't been merged to upstream `main`. 150: &"problem-specifications directory: '{probSpecsDir}'") 157: let remoteName = getNameOfRemote(probSpecsDir, upstreamHost, upstreamLocation) 163: &"problem-specifications directory: '{probSpecsDir}'") 167: &"problem-specifications directory: '{probSpecsDir}'") 169:proc init*(T: typedesc[ProbSpecsDir], conf: Conf): T = src/sync/sync.nim 3:import "."/[exercises, probspecs, sync_common, sync_docs, sync_filepaths, 86: let probSpecsDir = 88: ProbSpecsDir("this_will_not_be_used") 90: ProbSpecsDir.init(conf) 92: let psExercisesDir = probSpecsDir / "exercises" 119: let exercises = toSeq findExercises(conf, probSpecsDir) src/sync/sync_docs.nim 19: ProbSpecsSourceKind = enum 25: pssk: ProbSpecsSourceKind): string = 45: pssk: ProbSpecsSourceKind; 69:func toPath(pssk: ProbSpecsSourceKind): string = src/sync/sync_tests.nim 3:import "."/[exercises, probspecs] tests/all_tests.nim 7: test_probspecs, tests/test_binary.nim 2:import "."/[exec, helpers, lint/validators, sync/probspecs] 838: # Don't leave cached prob-specs dir in detached HEAD state. 842: test "can pull changes into cached prob-specs": 854: let probSpecsDir = ProbSpecsDir(psDir) # Don't use `init` (it performs extra setup). 855: let remoteName = getNameOfRemote(probSpecsDir, upstreamHost, upstreamLocation) 868: &"problem-specifications directory: '{probSpecsDir}'") tests/test_probspecs.nim 1:# This module contains tests for `src/probspecs.nim` 3:import "."/[cli, exec, sync/probspecs] 5:proc getProbSpecsExercises(probSpecsDir: ProbSpecsDir): Table[string, 6: seq[ProbSpecsTestCase]] = 8: ## each exercise in `probSpecsDir`. 9: let pattern = joinPath(probSpecsDir.string, "exercises", "*") 12: result[slug] = getCanonicalTests(probSpecsDir, slug) 22: let probSpecsDir = ProbSpecsDir.init(conf) 23: let probSpecsExercises = getProbSpecsExercises(probSpecsDir) 27: probSpecsExercises.len >= 116 30: let exercise = probSpecsExercises["accumulate"] 36: let firstTestCase = probSpecsExercises["accumulate"][0].JsonNode 52: let exercise = probSpecsExercises["acronym"] 58: let firstTestCase = probSpecsExercises["acronym"][0].JsonNode tests/test_sync.nim 508: # Don't leave cached prob-specs dir in detached HEAD state.

Be consistent with how it's written for --filepaths on a later line: complete -c configlet -n "__fish_seen_subcommand_from sync" -l filepaths -f -d 'Populate .meta/config.json "files" entry' The same inconsistency was in the `configlet --help` message. Blame ee7. That'll be fixed later.

Let's match the `fetch-configlet` script, which already exists on many machines, and indents with 2 spaces. We also use 2 spaces for the scripts everywhere else in this repo (that is, the scripts in `bin/` and `.github/bin/`). We could add something like `shfmt --diff -i 2 -ci -sr -kp` to CI in the future. $ shfmt --help usage: shfmt [flags] [path ...] shfmt formats shell programs. If the only argument is a dash ('-') or no arguments are given, standard input will be used. If a given path is a directory, all shell scripts found under that directory will be used. --version show version and exit -l, --list list files whose formatting differs from shfmt's -w, --write write result to file instead of stdout -d, --diff error with a diff when the formatting differs -s, --simplify simplify the code -mn, --minify minify the code to reduce its size (implies -s) Parser options: -ln, --language-dialect str bash/posix/mksh/bats, default "auto" -p, --posix shorthand for -ln=posix --filename str provide a name for the standard input file Printer options: -i, --indent uint 0 for tabs (default), >0 for number of spaces -bn, --binary-next-line binary ops like && and | may start a line -ci, --case-indent switch cases will be indented -sr, --space-redirects redirect operators will be followed by a space -kp, --keep-padding keep column alignment paddings -fn, --func-next-line function opening braces are placed on a separate line Utilities: -f, --find recursively find all shell files and print the paths --tojson print syntax tree to stdout as a typed JSON For more information, see 'man shfmt' and https://github.com/mvdan/sh.

ee7

Looks great. Thanks again, and sorry again for the delay in reviewing.

I hadn't used fish for a long time. Looks like they've improved compatibility a lot lately (e.g. support for && and ||, &>, and foo=bar echo $foo). And the completion approach for fish seems nice. Clear and self-documenting.

I took the liberty of pushing a few trivial commits (please yell at me if you object), and I've opened a bunch of follow-up issues. Minor things like #637 (I don't know if it's easy to address) and #642.

Merging.

glennj · 2022-08-04T17:10:57Z

Thanks @ee7!

ee7 · 2022-10-11T16:35:08Z

Released at last: https://github.com/exercism/configlet/releases/tag/4.0.0-beta.6

glennj requested a review from ErikSchierboom as a code owner July 8, 2022 23:47

ErikSchierboom requested a review from ee7 July 12, 2022 07:55

Add completions for bash and fish.

10dd1e0

Update create-artifact script to include completions in artifact.

glennj force-pushed the tab-completion branch from db12f3b to 10dd1e0 Compare July 12, 2022 12:58

ee7 reviewed Jul 12, 2022

View reviewed changes

missing --filepaths option for sync

12ff843

glennj requested a review from ee7 July 12, 2022 14:24

ee7 mentioned this pull request Jul 18, 2022

shell tab completions #615

Closed

ee7 mentioned this pull request Jul 29, 2022

completion: add configlet completion subcommand #631

Merged

ee7 added 5 commits August 4, 2022 10:20

create-artifact: exclude completions dir from release asset

2d6d7d8

Instead, it looks like we'll bake the completions into the configlet binary via a new subcommand [1]. [1] exercism#615 (comment)

configlet.fish: 'proc-spec' -> 'prob-spec'

7646c60

configlet.bash: fix grammar in comment

0ffe10d

This was referenced Aug 4, 2022

completions: add zsh #633

Closed

completions: add PowerShell #634

Open

fetch-configlet: detect user's shell and print instructions #635

Open

ee7 changed the title ~~Tab completion~~ completions: add completions for bash and fish Aug 4, 2022

ee7 approved these changes Aug 4, 2022

View reviewed changes

ee7 merged commit 690e9e2 into exercism:main Aug 4, 2022

ee7 mentioned this pull request Aug 4, 2022

completions: make uuid -n expect an argument #643

Merged

ee7 mentioned this pull request Aug 6, 2022

fetch-configlet: print instructions for installing completions #647

Draft

glennj deleted the tab-completion branch January 19, 2024 18:55

	shell=$(getent passwd "${USER}" \| cut -d: -f6)
	shell=$(getent passwd "${USER}" \| cut -d: -f7)

		elif [[ -f /etc/passwd ]]; then
		shell=$(awk -F: -v u="${USER}" '$1 == u {print $6}' /etc/passwd)

Uh oh!

completions: add completions for bash and fish #629

completions: add completions for bash and fish #629

Uh oh!

Conversation

glennj commented Jul 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ErikSchierboom commented Jul 12, 2022

Uh oh!

ee7 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ee7 Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

glennj Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

ee7 Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

ee7 Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glennj Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

glennj commented Jul 12, 2022

Uh oh!

ee7 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glennj commented Aug 4, 2022

Uh oh!

ee7 commented Oct 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

glennj commented Jul 8, 2022 •

edited

Loading

ee7 left a comment •

edited

Loading

ee7 Jul 12, 2022 •

edited

Loading

ee7 left a comment •

edited

Loading