-
Notifications
You must be signed in to change notification settings - Fork 19
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
Windows: ProcessBase.php line 172: Unable to decode output into JSON: Syntax error #56
Comments
I am having the same issue. How can I controll without manually? Is there any patch? |
Could someone provide a PR with a failing test on Windows? |
I'm pretty low on bandwidth to help out, but when things settle down I can give it a shot. |
It looks like we currently disable Windows testing of this function https://github.com/consolidation/site-process/blob/main/tests/SiteProcessTest.php#L289 |
My tiny obversation on this:
The I dont get it, because the |
If you enable the Windows Test for
I just googled an idea to remove the first additional surrounding double quotes: https://stackoverflow.com/a/9734805 if (Escape::isWindows()) {
$output = preg_replace('~^"?(.*?)"?$~', '$1', $output);
// Doubled double quotes were converted to \\".
// Revert to double quote.
$output = str_replace('\\"', '"', $output);
// Revert of doubled backslashes.
$output = preg_replace('#\\\\{2}#', '\\', $output);
}
With that single line I have 7 of 8 passing tests :D The failing test is the first one:
But I dont think that this is relevant to the JSON Decode. |
I created a Drush PR so we don't use those Windows backslashes in status output (which is what |
yeah I think this is a nice avoidance for the baclslashed topic. I remind that there is also a case for double quotes in the update messages.
|
Still getting this with drush 11.6.0 (in Windows); which I think was merged into 11.x months before the 11.6.0 release. |
Still getting this with drush 12.3.4. |
Thanks for continuing to ping this issue. Activity shows which issues folks still care about. Might have time to look at this over the holidays, but things have been pretty busy, so no promises. A PR with tests would be very welcome here. |
Doubt i'll come up with a PR but i'll see about posting the point where it fails. I suspect this is caused by specific contrib modules (as sunlix suggests) and the current fix is likely not generic enough to cover all cases. |
From the simple_sitemap example above, it seems that better encoding would solve one large subset of the problems being encountered here. |
Although maybe the simple_sitemap example might be an encoding problem in the simple_sitemap module, which would have to be fixed there, not here. |
The failing simple_sitemap example fails because it has no escaped double quotes sequence for JSON. |
What is the site-process command you're running that produces the un-escaped output shown above? |
just |
our output leading to fail:
|
Describe the bug or behavior
Running
drush drupal:directory
always results in an exception:To Reproduce
See above.
Expected behavior
Not a horrible error. :P
Actual behavior
See output above.
Workaround
After a bit of digging and trial and error, I discovered that commenting out line 162 in ProcessBase.php fixes the issue for some reason, i.e. this line:
Then running the same Drush command completes successfully:
System Configuration
Additional information
Possible related, posted comment: drush-ops/drush#4281
The text was updated successfully, but these errors were encountered: