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 Windows Support for tinker() #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fschirinzi
Copy link

I could make it work, that the app is opened in Windows.
The only thing that doesn't work, for now, is that the variables are not populated.

I hope my pull request helps to bring this feature as fast as possible to Windows... because it's awesome!!!

@mpociot
Copy link
Member

mpociot commented Feb 14, 2020

Nice job, thank you!
I'll take a look at the app implementation before merging this one :)

src/helper.php Outdated Show resolved Hide resolved
@@ -17,7 +17,8 @@
],
"require": {
"php": "^7.2",
"illuminate/support": "5.6.*|5.7.*|5.8.*|^6.0"
"illuminate/support": "5.6.*|5.7.*|5.8.*|^6.0",
"symfony/process": "^5.0"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "symfony/process": "^4.0|^5.0"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right.

if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
$logPath = base_path() . '\\storage\\logs\\tinkerwell-' . date("Y-m-d") . '.log';

// For Windows || ATTENTION ... I added a ^ before the & to escape it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice comment! This is an annoying quirk of Windows that I only discovered the other day. 👍

Co-Authored-By: Owen Voke <owzie123@gmail.com>
@sschlein
Copy link
Member

Hey, sorry that this hasn't been merged so far. Could you please update the PR so that it solves the merge conflicts and works with Laravel 7 too?

We will take care about this after this has been solved.

@sschlein sschlein linked an issue Jul 20, 2020 that may be closed by this pull request
@fschirinzi
Copy link
Author

Unfortunately, I cannot make it to work.

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.

Unable to fork [open ...]
4 participants