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

[9.x] Update dd function when running on virtualized dev environments #44360

Closed
wants to merge 2 commits into from
Closed

Conversation

jevouslue
Copy link

@jevouslue jevouslue commented Sep 29, 2022

This PR enhances the recently added feature, dd function output (#44211).
The great feature "clickable file path" doesn't work when running on virtualized dev environments, including Laravel Sail.

You just add the app.editor_project_path configuration, and the source link will work properly even on your virtualized dev environments.

config/app.php

<?php

use Illuminate\Support\Facades\Facade;

return [
    'editor' => env('EDITOR'),  // UPDATE
    'editor_project_path' => env('EDITOR_PROJECT_PATH'),  // ADD!

    // the other settings
];

Of course, this is optional.

And I also think the params should be in the .env file.
.env(example)

EDITOR=phpstorm
EDITOR_PROJECT_PATH=/Users/username/Desktop/example-app

This is my first PR for any OSS projects, maybe something weird...

@jevouslue jevouslue changed the title Update dd function when running on virtualized dev environments [9.x] Update dd function when running on virtualized dev environments Sep 29, 2022
@@ -133,10 +133,11 @@ protected function getDumpSourceContent()
$source = sprintf('%s%s', $relativeFile, is_null($line) ? '' : ":$line");

if ($editor = $this->editor()) {
$editorProjectPath = $this->editorProjectPath();
Copy link
Member

Choose a reason for hiding this comment

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

There is no way to infer the project's path, so the user don't have to hardcode it on his configuration file / environment variables?

Copy link
Contributor

@fabio-ivona fabio-ivona Sep 29, 2022

Choose a reason for hiding this comment

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

I don't think you can get the host path from inside a docker container. Also, this feature would be useful for opening local files from a dd() on a remote server

Maybe it could be useful to use a single env value for both ingnition and dd() features (for ignition is currently used IGNITION_LOCAL_SITES_PATH)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, there is no way to automatically infer the project's path as the editor that the users use.
That's why I have the user hardcode it.

Unfortunately, I have no idea.
Do you have any solution?

Copy link
Author

@jevouslue jevouslue Sep 29, 2022

Choose a reason for hiding this comment

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

@fabio-ivona
It sounds good!
IGNITION_EDITOR and IGNITION_LOCAL_SITES_PATH is suitable for them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would use a more general key, like (EDITOR_BASE_PATH) and ask to Spatie to support that one as well

using "IGNITION" for this feature doesn't seem consistent to me

@sirmathays
Copy link

sirmathays commented Sep 29, 2022

I made a pull request (#44378) that seems to overlap with this one at least a little bit:

// config/app.php

return [
    // ....
    'editor' => 'vscode',
    'editor_href' => 'vscode-remote/wsl+Ubuntu-20.04{file}:{line}'
    // ...
];

Then in HtmlDumper the {file} and {line} gets replaced with the actual values. This approach will make the url work with VSCode and WSL.

@nunomaduro
Copy link
Member

Closing this pull request, as I will be working on this format: #44378 (comment).

@nunomaduro
Copy link
Member

@jevouslue Do you mind of try to use the branch #44406 locally, and let me know if it works for you in both the terminal and browser please.

@jevouslue
Copy link
Author

jevouslue commented Oct 1, 2022

@nunomaduro
For me, the following configuration worked on my Docker via Laravel Sail 👍

'editor' => [
    'name' => 'phpstorm',
    'base_path' => '/Users/username/Desktop/example-app',
],

let me know if it works for you in both the terminal and browser please.

In the terminal, it just displays the file path as before, and it's non-clickable, right? (I mean the color)
then it works well!

FYI It seems different when I enable the editor setting.
Screen Shot 2022-10-01 at 10 41 41

You're always doing great!
Thank you so much!
I can't wait for it to be merged.

@fabio-ivona
Copy link
Contributor

@jevouslue isn't the second one clickable?

which terminal are you using?

@jevouslue
Copy link
Author

@fabio-ivona
Sorry, I just misunderstood.
(I forget to press a control key...)

The second one is also clickable!

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.

5 participants