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

adopt clack #627

Merged
merged 14 commits into from
Jan 30, 2024
Merged

adopt clack #627

merged 14 commits into from
Jan 30, 2024

Conversation

trebor
Copy link
Contributor

@trebor trebor commented Jan 29, 2024

changes:

  • replace prompts with clack
  • add empty template
  • simlink common files between default and empty files
  • improve template README.md
  • refactor prompts

@trebor
Copy link
Contributor Author

trebor commented Jan 29, 2024

@mbostock there isn't an easy fix for the unit test, i'm hunting around for an alternative, but no luck so far. i could perhaps make a moch clack or something like that. that said, i don't want to hold this show up. so not sure if i should dive deeper on the unit test or bypass it for now.

@mythmon
Copy link
Member

mythmon commented Jan 30, 2024

@trebor I spent some time today trying to use Clack elsewhere. I wasn't able to find any way to mock it directly. Instead I'm using the effect pattern and isolating all the Clack call into funtions that I swap out in tests.

@mootari
Copy link
Member

mootari commented Jan 30, 2024

(Referencing the 4 months old open PR bombshell-dev/clack#163 since it hasn't been mentioned yet from what I could tell.)

@mbostock
Copy link
Member

I’m investigating testing now, and then I’ll approve (hopefully with a workaround, but we’ll see).

src/create.ts Outdated Show resolved Hide resolved
@trebor
Copy link
Contributor Author

trebor commented Jan 30, 2024

btw: i ran tests with create and everything ran smooth.

@mbostock
Copy link
Member

I implemented some simple mocks for clack. It should do the trick for now.

mbostock and others added 2 commits January 29, 2024 17:12
Co-authored-by: Robert Harris <trebor@observablehq.com>
@trebor
Copy link
Contributor Author

trebor commented Jan 30, 2024

great, thank you! we ready to merge?

@mbostock
Copy link
Member

I found some more bugs in path validation (like, it didn’t support creating nested folders, or handle the path being a file). I’m testing some more and will approve & merge shortly.

@mbostock mbostock enabled auto-merge (squash) January 30, 2024 01:36
@mbostock mbostock merged commit d02782e into main Jan 30, 2024
2 checks passed
@mbostock mbostock deleted the trebor/use-clack-prompting branch January 30, 2024 01:36
@mcglincy
Copy link
Contributor

woo, excited to use clack :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants