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

Add ability to specify --dependency style deps #105

Merged
merged 1 commit into from
Apr 15, 2023

Conversation

siefkenj
Copy link
Contributor

Fixes #100 and adds a test to make sure dependencies work in the future.

Copy link
Contributor

@hildjj hildjj left a comment

Choose a reason for hiding this comment

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

Up to you if testing on Windows is important or not.

const barSource = path.join(ROOT_DIR, 'examples/bar.ts');
const barDest = path.join(ROOT_DIR, 'output/bar.ts');
// Copy the dependency to the output directory
await exec(`cp "${barSource}" "${barDest}"`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there's something I'm not seeing, won't this mostly fail on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does cp not work on Windows? I used double quotes " specifically so that Windows wouldn't complain...

Copy link
Contributor

Choose a reason for hiding this comment

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

I use bash on windows, that's why it worked for me. Otherwise, you need to use copy.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can use copyfiles to have it work mostly everywhere, or just rewrite the test to not need copying.

{
// Create the parser
const { stdout, stderr } = await exec(
`npx peggy --plugin "${PLUGIN_PATH}" --dependency foo:./bar -o "${outTsName}" "${GRAMMAR_FILE}"`
Copy link
Contributor

Choose a reason for hiding this comment

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

It's worth trying with --dependencies "{\"foo\": \"./bar\"}" or a config file with a dependencies key, as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is okay, since Peggy merges --dependency and --dependencies together for us, we'll trust Peggy to do it right.

@pjmolina pjmolina merged commit 55f26aa into metadevpro:master Apr 15, 2023
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.

--dependency not working any more?
3 participants