-
Notifications
You must be signed in to change notification settings - Fork 54
Studio CLI: Add support for xdebug beta flag #2316
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
Conversation
nightnei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- If some site has xdebux already - then everything good

- I tried to disable Xdebug in UI - everything behaves as normal, but it's not actually disabled - in UI, it's still enabled, and it's still indeed enabled. It seems we have regression.
- Turning on/off xdebug via cli - doesn't reflect changes in Studio, hovever terminal doesn't print errors.
Maybe it's again solely local issues, however, I removed node_modules and did everything from scratch. @bcotrim, coudl you please double-check these two points?
Thanks for the review and feedback @nightnei |
I see 👍 Let's merge that PR and rebase this one, and I will re-test |
…cli-xdebug-support
I updated the branch with the latest changes. Everything works for me now, can you retest please? |
gavande1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…cli-xdebug-support # Conflicts: # cli/index.ts
|
@nightnei @gavande1 @fredrikekelund after we merged #2355 I updated this PR to merge the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this and works as expected. LGTM 👍
I had run following command to enable and disable xdebug.
// Enable
node dist/cli/main.js site set --xdebug --path /tmp/xdebug-test
// Disable
node dist/cli/main.js site set --xdebug=false --path /tmp/xdebug-test
Perhaps, It would be great idea to update test instructions for posterity. cc @bcotrim
Nice catch, thank you! Updated |
fredrikekelund
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍


Related issues
Proposed Changes
studio site set-xdebug <enable>command to enable/disable xdebug for a sitexdebugSupport) and per-site setting (enableXdebug)isXdebugBetaEnabled()to check the beta feature flagTesting Instructions
npm run cli:buildnode dist/cli/main.js site create --name test-xdebug --path /tmp/xdebug-testnode dist/cli/main.js site set --xdebug true --path /tmp/xdebug-testnode dist/cli/main.js site start --path /tmp/xdebug-testnode dist/cli/main.js site set --xdebug false --path /tmp/xdebug-testEdge cases to verify:
Pre-merge Checklist