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

chore: wrap the projectDir in quotes if it contains spaces #6594

Merged
merged 4 commits into from
Mar 21, 2023

Conversation

btea
Copy link
Contributor

@btea btea commented Mar 19, 2023

Changes

When outputting projectDir in the terminal, if it contains spaces, wrap it in quotes.

image

Testing

Docs

@changeset-bot
Copy link

changeset-bot bot commented Mar 19, 2023

🦋 Changeset detected

Latest commit: 8477bd8

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: create-astro Related to the `create-astro` package (scope) label Mar 19, 2023
Comment on lines 124 to 129
if (projectDir.includes(' ')) {
projectDir = `"${projectDir}"`;
}
const enter = [
`\n${prefix}Enter your project directory using`,
color.cyan(`cd ./${projectDir}`, ''),
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use a real shell-quoting function like quote(["cd", `./${projectDir}`]) from the shell-quote library, rather than a one-off hack for just one of many shell metacharacters that might be misinterpreted.

Copy link
Member

Choose a reason for hiding this comment

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

Since this only shows a hint of what to do next, and it's unlikely that someone would name their project with $ and ' etc, I think this is enough. It feels overkill to bring in a library for this.

.changeset/tall-beans-own.md Outdated Show resolved Hide resolved
packages/create-astro/src/messages.ts Outdated Show resolved Hide resolved
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Could you please fill the template around testing? If you tested manually, perhaps you could attach a screenshot/video

@btea
Copy link
Contributor Author

btea commented Mar 21, 2023

@ematipico I added a screenshot #6594 (comment)

@ematipico ematipico merged commit a661907 into withastro:main Mar 21, 2023
@btea btea deleted the chore/wrap-projectDir branch March 21, 2023 09:28
@astrobot-houston astrobot-houston mentioned this pull request Mar 21, 2023
bholmesdev added a commit that referenced this pull request Mar 21, 2023
bholmesdev added a commit that referenced this pull request Mar 21, 2023
* [ci] release

* nit: typo in #6594 changeset

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Ben Holmes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: create-astro Related to the `create-astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants