fix: resolve hanging issue when piping installer through curl#11
fix: resolve hanging issue when piping installer through curl#11ivangrynenko merged 1 commit intomainfrom
Conversation
- Fixed parseArguments() not being properly merged when piped - Added handling for missing -- separator in argv when piped - The script now correctly processes --ws and other options when used via curl
There was a problem hiding this comment.
Summary of Changes
Hello @ivangrynenko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves an issue where the install.php script would hang when piped through curl and provided with command-line arguments. The fix ensures that arguments are correctly parsed and processed in such scenarios, allowing the installer to proceed as expected.
Highlights
- Argument Parsing Fix: Resolved an issue where the installer script would hang when piped through curl and provided with command-line arguments.
- Option Merging: Ensured that options parsed from STDIN are correctly merged with the default options, preventing argument loss.
- Piped Input Handling: Improved argument parsing logic within parseArguments() to correctly identify and process arguments when the script is executed via a pipe (e.g., curl | php), even when the -- separator is absent.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request addresses an issue causing the installer to hang when piped through curl with arguments. The primary fix, which involves correctly merging parsed options with default values, is a solid improvement and resolves the core problem. I've identified a small piece of redundant logic in the argument parsing section and have left a comment with a suggestion to simplify it. Otherwise, the changes look good and effectively solve the reported issue.
| // If no separator found and we're being piped, assume all args after argv[0] are ours | ||
| if (!$found_separator && $argc > 1 && $argv[0] === 'Standard input code') { | ||
| $start_index = 1; | ||
| } |
There was a problem hiding this comment.
This conditional block appears to be redundant. The $start_index variable is initialized to 1 on line 1167. If the preceding for loop does not find a -- separator, $found_separator will be false and $start_index will still be 1. This if block then re-assigns $start_index to 1, which has no effect on the program's logic.
This block and its preceding comment can be safely removed to improve code clarity. If there was a different intention here that I'm missing, please clarify.
fix: resolve hanging issue when piping installer through curl
Summary
curl ... | php -- --ws)Problem
When running the installer with
curl -s https://raw.githubusercontent.com/ivangrynenko/cursor-rules/main/install.php | php -- --ws, the script would hang without doing anything.Root Cause
stream_isatty(STDIN)returns false, triggering theparseArguments()functionparseArguments()were not being merged back into the main options arrayparseArguments()function wasn't handling the case where there's no--separator in argvSolution
--separator (lines 1177-1180)Test Plan
cat install.php | php -- --ws --debug