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

Allow workspaceRoot variable in pathToFlow #173

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

rattrayalex
Copy link
Contributor

flow.useNPMPackagedFlow wasn't adequate for my use-cases (node_modules not at root, a copy for flow-bin outside of node-modules, etc).

I was resorting to this hack: "postinstall": "cp .vscode/settings.example.json .vscode/settings.json && sed -i '' \"s ROOT_REPLACE_ME $(pwd) \" .vscode/settings.json"

Ideally this would be solved by vscode; there is an open issue: microsoft/vscode#2809 however it seems harmless to add this.

I tested locally with:

  • setting defined correctly ("flow.pathToFlow": "${workspaceRoot}/my-flow")
  • setting defined incorrectly ("flow.pathToFlow": "${workspaceRoot}/my-nonexistent-flow")
  • setting undefined (checking that .config('pathToFlow').replace does not throw)

@orta
Copy link
Contributor

orta commented Oct 23, 2017

This is 👍 from me, any chance you can add some docs in the README for this first?

@rattrayalex
Copy link
Contributor Author

Hmm, I won't be able to get to that shortly... I can try to get to it within a few days.

@rattrayalex
Copy link
Contributor Author

(Feel free to merge without if you're comfortable with that, I'm happy to submit a follow-on PR if that's okay by you)

@orta
Copy link
Contributor

orta commented Oct 24, 2017

👍 that's fine by me, yeah

@orta orta merged commit 14e25c5 into flow:master Oct 24, 2017
@rattrayalex rattrayalex deleted the workspace-root branch October 26, 2017 16:57
rattrayalex added a commit to rattrayalex/flow-for-vscode that referenced this pull request Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants