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 support for more options #12

Merged
merged 7 commits into from
Oct 13, 2022
Merged

Add support for more options #12

merged 7 commits into from
Oct 13, 2022

Conversation

dnkmdg
Copy link
Contributor

@dnkmdg dnkmdg commented Oct 13, 2022

When copying request from Chrome DevTools the options --insecure and --compressed are sometimes present, and was not recognized by the package.

The --insecure option should primarily be present when posting/fetching data from a non-SSL endpoint.
The --compressed option indicates that cURL accepts any encoded response, equivalent to CURLOPT_ENCODING = ""

I added support for these two in the options gatherer. They won't have any effect on the standard HTTP call, thus are simply ignored.

Removed notes about #8

@dnkmdg dnkmdg changed the title Add support for options from Chrome Add support for more options Oct 13, 2022
@jasonmccreary
Copy link
Collaborator

Thanks. A few things:

  • This includes changes to different issues. Ideally these would be in separate PRs - one for the Chrome dev tools options and one for the --data-raw issue.
  • There are no tests for the ignored options. Check the recently added --location option for an example.

@dnkmdg
Copy link
Contributor Author

dnkmdg commented Oct 13, 2022

Agreed on separate PRs, figured I'd include them both since they don't really affect anything outside of CurlCommand::class. I can remove the --data-raw-part and put in a separate PR.

There are tests however, both fixtures are available as with-compressed-option and with-insecure-option. Or should there be any more specific tests?

@jasonmccreary
Copy link
Collaborator

Thanks. If you can remove the left over data-raw fixtures from this PR, I'll get it merged.

@jasonmccreary jasonmccreary merged commit c011a2e into laravel-shift:main Oct 13, 2022
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.

2 participants