Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Added a default ide file link web view#19973
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
e9d1c13 tofbd4a6fCompare0f180bc toa0267a7Compare| 'macvim' =>'mvim://open?url=file://%%f&line=%%l', | ||
| 'emacs' =>'emacs://open?url=file://%%f&line=%%l', | ||
| 'sublime' =>'subl://open?url=file://%%f&line=%%l', | ||
| 'symfony' =>'/_profiler/open?file=%%f&line=%%l#line%%l#'.json_encode(dirname($container->getParameter('kernel.root_dir')).DIRECTORY_SEPARATOR).':""', |
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.
the app root doesn't always match the server root
| $filename =dirname($this->kernelRootDir).DIRECTORY_SEPARATOR.$file; | ||
| if (preg_match("'(^|[/\\\\])\.\.?([/\\\\]|$)'",$file) || !is_readable($filename)) { |
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.
$filename => $file, we shouldn't leak the kernel root dir
a0267a7 to06ad113Comparero0NL commentedSep 20, 2016
@jeremyFreeAgent can we open this page relatively simple in a modal overlay? Not leaving the current page. Simple iframe loading would be just fine i guess.. |
fabpot commentedSep 23, 2016
I like this. Can you take@nicolas-grekas comments into account? |
41fc036 to6a0340eCompareb127be7 to3bf030aCompare3bf030a to837fdf3Compare| } | ||
| $profilerController =$container->getDefinition('web_profiler.controller.profiler'); | ||
| $profilerController->replaceArgument(6,dirname(dirname(dirname(dirname(dirname(dirname(dirname(dirname(__DIR__))))))))); |
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.
does work with standalone web profiler bundle (from subtree split)
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.
does or does not?
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.
what about taking the common dir prefix ofkernel.root_dir &__DIR__
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.
does not!
| private$toolbarPosition; | ||
| private$cspHandler; | ||
| private$kernelRootDir; | ||
| private$rootDir; |
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.
baseDir?
| returnarray( | ||
| $request->getSchemeAndHttpHost().$request->getBaseUrl().$this->urlFormat, | ||
| dirname($this->rootDir).DIRECTORY_SEPARATOR,'', | ||
| $this->rootDir.DIRECTORY_SEPARATOR,'', |
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.
baseDir?
nicolas-grekas commentedOct 25, 2016
👍 (failure unrelated, know composer bug) |
jeremyFreeAgent commentedOct 25, 2016
I've made the changes in the Silex WebProfiler PR too. |
| */ | ||
| publicfunctionopenAction(Request$request) | ||
| { | ||
| if (null ===$this->baseDir) { |
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.
Should be much more explicit than that with a bit of context on how to fix this.
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.
Actually the extension replace the argument baseDir should be always set and that exception may not be thrown, ever. Perhaps we can remove that test here. What do you think?
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.
Indeed, let's keep it as is
fabpot commentedOct 25, 2016
Thank you@jeremyFreeAgent. |
This PR was merged into the 3.2-dev branch.Discussion----------Added a default ide file link web view| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -When having no `framework.ide` configured or `framework.ide = symfony` the file link open the source in a web view (eg `_profiler/open?file=/src/AppBundle/Controller/DefaultController.php&line=50#line50`).Commits-------ba6bcca Added a default ide file link web view
…reeAgent)This PR was merged into the 2.0.x-dev branch.Discussion----------Added the Symfony default ide file link web viewThis add the feature added bysymfony/symfony#19973Commits-------7e7f062 Added the Symfony default ide file link web view
…r (jeremyFreeAgent, javiereguiluz)This PR was merged into the master branch.Discussion----------Updated the ide option to use the default open in browserRelated tosymfony/symfony#19973Commits-------2ee69c6 Added help about the framework.ide null value behaviour3f4fcc7 Updated the help note about framework.ide338e423 Updated the ide option to use the default open in browser
mtibben commentedNov 8, 2017
FYI@jeremyFreeAgent I've found a small problem with the open link URL, see#24868 |
…r (jeremyFreeAgent, javiereguiluz)This PR was merged into the master branch.Discussion----------Updated the ide option to use the default open in browserRelated tosymfony/symfony#19973Commits-------2ee69c6 Added help about the framework.ide null value behaviour3f4fcc7 Updated the help note about framework.ide338e423 Updated the ide option to use the default open in browser
…r (jeremyFreeAgent, javiereguiluz)This PR was merged into the master branch.Discussion----------Updated the ide option to use the default open in browserRelated tosymfony/symfony#19973Commits-------2ee69c6 Added help about the framework.ide null value behaviour3f4fcc7 Updated the help note about framework.ide338e423 Updated the ide option to use the default open in browser
…r (jeremyFreeAgent, javiereguiluz)This PR was merged into the master branch.Discussion----------Updated the ide option to use the default open in browserRelated tosymfony/symfony#19973Commits-------2ee69c6 Added help about the framework.ide null value behaviour3f4fcc7 Updated the help note about framework.ide338e423 Updated the ide option to use the default open in browser
Uh oh!
There was an error while loading.Please reload this page.
When having no
framework.ideconfigured orframework.ide = symfonythe file link open the source in a web view (eg_profiler/open?file=/src/AppBundle/Controller/DefaultController.php&line=50#line50).