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

Fix dev server run error when path contains spaces #609

Merged
merged 3 commits into from
May 27, 2020

Conversation

thedavidprice
Copy link
Contributor

Related #596

This addresses two separate problems when paths contain spaces:

  1. errors when checking if dir or file exists: the previous implementation gave a false-negative if the path contained a space
  2. running the commands with concurrently: running commands will fail if the path spaces are not sanitized

Solutions:

  • for dir and file validation checks, uses getPaths()
  • in case of DB, now checks if schema.prisma exists
  • in case of effectively setting the working directory for concurrently (e.g. cd api/), we still need to use the "sanitize-path" method replace(' ', '\\ '), otherwise spaces still cause the run(s) to fail

@thedavidprice
Copy link
Contributor Author

@peterp I've been testing on Windows Powershell. And I'm getting some strange behavior for paths using getPaths(), both with and without spaces, specifically:

  • getPaths().api.base returns undefinded
  • getPaths().web.base returns undefinded
    ...BUT...
  • getPaths().api.dbSchema returns successfully (with or without space) as well as
  • getPaths().web.srcreturns successfully
  • getPaths().api.srcreturns successfully

So, more ¯_(ツ)_/¯

@thedavidprice thedavidprice requested a review from peterp May 27, 2020 02:36
@thedavidprice
Copy link
Contributor Author

Last change --> added " to the bash cd command to support both Windows and Mac in case of space in path.

@peterp Per my tests, this is working on both Windows and Mac with/without spaces in paths. I would appreciate your eyes on this before merging. It's a messy one.

Copy link
Contributor

@peterp peterp left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@thedavidprice
Copy link
Contributor Author

@peterp Cool, I'll merge now.

Just want to make sure I (re)highlight this strange behavior on Windows Powershell 'cause it might come back to bite us:

  • getPaths().api.base returns undefined
  • getPaths().web.base returns undefined

I'm about to test something similar on build.js. TBD.

@thedavidprice thedavidprice merged commit 726ff2b into master May 27, 2020
@thedavidprice thedavidprice deleted the dsp-fix-dev-server-error-with-path-space branch May 27, 2020 06:07
@thedavidprice thedavidprice added this to the next release milestone May 27, 2020
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.

None yet

2 participants