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 invoke example for Windows CMD #604

Merged
merged 7 commits into from
Feb 22, 2023

Conversation

edmuthiah
Copy link
Contributor

@edmuthiah edmuthiah commented Feb 18, 2023

Description of changes:

Currently the invoke example cargo lambda invoke rust-lambda --data-ascii '{ "command": "hi" }' does not work on Windows and returns the following error because of the way that Windows handles single quotes. Trying to invoke the command as shown in the readme returns: error: Found argument 'command:' which wasn't expected, or isn't valid in this context

This change the example invoke command to run for Windows, Mac and Linux.

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

Edit: Updated the PR name to include CMD

@winstxnhdw
Copy link
Contributor

winstxnhdw commented Feb 18, 2023

This isn't a Windows-specific issue, but a shell-specific issue. More specifically, CMD. If we do go ahead with this change, however, please update the rest of the examples to follow this convention.

@edmuthiah edmuthiah changed the title Fix invoke example for Windows Fix invoke example for Windows CMD Feb 18, 2023
@edmuthiah
Copy link
Contributor Author

Done 👍🏽

@bnusunny
Copy link
Contributor

bnusunny commented Feb 20, 2023

I recommend to keep them seperated, and show examples for both Linux/MacOS and Windows.

@winstxnhdw
Copy link
Contributor

winstxnhdw commented Feb 20, 2023

I recommend to keep them seperated, and show examples for both Linux/MacOS and Windows.

What is the benefit to doing this? Escaping the double quotes should work on most modern shells.

@bnusunny
Copy link
Contributor

Linux and MacOs users could use a simpler format.

@winstxnhdw
Copy link
Contributor

How can we do this without cluttering the docs?

@bnusunny
Copy link
Contributor

Add one section to talk about it. There is an example in AWS CLI doc.

@edmuthiah
Copy link
Contributor Author

Done 👍🏽 Let me know if it's in an appropriate spot.

@bnusunny
Copy link
Contributor

I suggest to put this note close to the first apperance of cargo lambda invoke.

@edmuthiah
Copy link
Contributor Author

Done 👍🏽

@bnusunny bnusunny merged commit 54a3d93 into awslabs:main Feb 22, 2023
@edmuthiah edmuthiah deleted the fix-windows-invoke branch February 22, 2023 06:01
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.

3 participants