Added ability to config the trace file and line number #162

thiagotalma · 2016-11-07T13:16:30Z

Q	A
Is bugfix?	no
New feature?	no
Breaks BC?	no
Tests pass?	no

dynasource · 2016-11-07T13:20:34Z

this would be more consistent indeed. Can you add a link to this convention in this topic?

bashkarev · 2016-11-07T14:28:46Z

It will be more universal. + closes #144

[
    'modules' => [
        'debug' => [
            'class' => 'yii\debug\Module',
            'editor' => ['phpstorm:https://open?file={file}&line={line}', '{file}:{line}']
        ],
    ],
]

public function editorLink($trace)
{
    if ($this->editor === null) {
        return "{$trace['file']}:{$trace['line']}";
    }
    if (is_string($this->editor)) {
        return strtr($this->editor, ['{file}' => $trace['file'], '{line}' => $trace['line']]);
    }
    return Html::a(strtr($this->editor[0], ['{file}' => $trace['file'], '{line}' => $trace['line']]), strtr($this->editor[1], ['{file}' => $trace['file'], '{line}' => $trace['line']]));
}

samdark

Need a changelog.

thiagotalma · 2016-11-07T16:59:41Z

@samdark,, I made the changes that colleagues asked. Please check again.

dynasource · 2016-11-07T17:30:46Z

Panel.php

+ /**
+ * @param array $trace The log trace
+ *
+ * @return string the trace line


missing @SInCE

dynasource · 2016-11-07T17:31:19Z

Panel.php

+ if (is_callable($this->module->traceLink)) {
+ return call_user_func($this->module->traceLink, $trace, $this);
+ }
+


blank line may be removed.

dynasource · 2016-11-07T17:32:13Z

Module.php

@@ -91,6 +91,19 @@ class Module extends \yii\base\Module implements BootstrapInterface
 * You may want to enable the debug logs if you want to investigate how the debug module itself works.
 */
 public $enableDebugLogs = false;
+ /**
+ * @var mixed the string with placeholders to be be substituted or an anonymous function that returns the trace line string.


its unclear how this string should look like?

yes, an example would be good.

dynasource · 2016-11-07T17:32:47Z

Panel.php

@@ -114,4 +114,22 @@ public function getUrl($additionalParams = null)

 return Url::toRoute($route);
 }
+
+ /**
+ * @param array $trace The log trace


missing method description

bashkarev · 2016-11-07T17:33:28Z

What do you think?

$links = [
    'textmate' => 'txmt:https://open?url={file}&line={line}',
    'macvim' => 'mvim:https://open?url={file}&line={line}',
    'emacs' => 'emacs:https://open?url={file}&line={line}',
    'sublime' => 'subl:https://open?url={file}&line={line}',
    'phpstorm' => 'phpstorm:https://open?url={file}&line={line}',
];

[
    'modules' => [
        'debug' => [
            'class' => 'yii\debug\Module',
            'ide' =>'phpstorm'
        ],
    ],
]

dynasource · 2016-11-07T17:34:25Z

views/default/panels/db/detail.php

@@ -98,7 +98,7 @@
 function debug_db_detail() {
 $('.db-explain a').on('click', function(e) {
 e.preventDefault();
- 
+


are these lines added accidentally?

It's automatic PhpStorm's removal of whitespace on blank lines. I agree that it should not be in this commit...

dynasource

thanks for including the traceLink functionality. Looking forward to have this integrated with PhpStorm.

samdark · 2016-11-07T17:37:33Z

@bashkarev this is interesting. Presets for common IDEs and editors could be very handy. Alternatively it could be put into readme.

bashkarev · 2016-11-07T18:30:27Z

please add options text.
see example for \yii\web\ErrorHandler.php

public function traceLink($file, $line)
{
    /**
     * @var $debug \yii\debug\Module
     */
    if (
        $file !== null
        && $line !== null
        && ($debug = Yii::$app->getModule('debug')) !== null
    ) {
        return $debug->traceLink([
            'file' => $file,
            'line' => $line,
            'text' => 'open file' //!!!
        ]);
    }
    return null;
}

thiagotalma · 2016-11-07T19:07:19Z

adjustments done!

dynasource

well done

bashkarev · 2016-11-07T19:53:49Z

views/default/panels/db/detail.php

- 'item' => function ($trace) {
- return "<li>{$trace['file']} ({$trace['line']})</li>";
+ 'item' => function ($trace) use ($panel) {
+ return '<li>' . $panel->traceLink($trace) . '</li>';


if set 'traceLink' => 'phpstorm:https://open?url={file}&line={line}', this not link

@bashkarev really! my bad....
fixed in README

samdark · 2016-11-07T20:02:07Z

README.md

+ ...
+];
+```
+


I suggest putting common IDE strings here.

@samdark In fact, there is no a list of possible settings per IDE.

The user is who sets the string when it does the setup in the operating system. There is a link that talks about this in the example I quoted: https://pla.nette.org/en/how-open-files-in-ide-from-debugger

correct, shortly described at #144 (comment)

example symfony

bashkarev · 2016-11-07T20:15:42Z

again for about option text

public function traceLink($options)
{
    if (!isset($options['text'])) {
        $options['text'] = "{$options['file']}:{$options['line']}";
    }
    if ($this->module->traceLink === null) {
        return $options['text'];
    }
    if (is_callable($this->module->traceLink)) {
        return call_user_func($this->module->traceLink, $options, $this);
    }
    if (is_string($this->module->traceLink)) {
        return strtr($this->module->traceLink, ['{file}' => $options['file'], '{line}' => $options['line'], '{text}' => $options['text']]);
    }
    return null;
}

$debug->traceLink(['file'=>'...','line'=>1,'text'=>'<svg></svg>'])

dynasource · 2016-11-07T20:36:25Z

alright, are you sure everything is working correctly? Thats 'traceLink' method has reached a certain level of complexity that it deserves some unit tests.

thiagotalma · 2016-11-07T20:38:18Z

Thats 'traceLink' method has reached a certain level of complexity that it deserves some unit tests.

@bashkarev you can help with tests?

not ready yet

dynasource · 2016-11-09T22:19:36Z

alright. I've commited some fixes & tests.

It also tried to integrate it with PhpStorm on a windows machine. That was quite some work and the good news is thats its working. You need several patches for that:

install a url protocol to make the links work in the browser
create the correct links with files & line numbers
use a bat file to link to PhpStorm
without having any CMD windows left open

Took a few hours ;) but it works now. Unfortunately yet only with files. With the line numbers, I do not see any activity in the PhpStorm. Its very likely a bug on their side.

samdark · 2016-11-09T22:35:19Z

They have an issue tracker: http:https://youtrack.jetbrains.net/issues/WI

thiagotalma · 2016-11-09T23:24:56Z

@dynasource I got it with a minimum of work.
I've created a gist for you to try to use, just copy the files to the "C:\Program Files\PhpStorm Protocol (Win)" folder and run .reg

https://gist.github.com/thiagotalma/fa5a54e1e34133f78f4538eb96c87ae8

EDIT:
Change the .js file according to the configuration of your machine.
This solution was taken from the link I included in the README.

dynasource · 2016-11-10T08:25:03Z

my solution is a little different and smaller. Its uses a bat file so you only have to put it to the registry once (and change the bat file if PhpStorm upgrades):

Windows Registry Editor Version 5.00

[HKEY_CLASSES_ROOT\phpstorm]
@="\"URL:phpstorm Protocol\""
"URL Protocol"=""

[HKEY_CLASSES_ROOT\phpstorm\shell]

[HKEY_CLASSES_ROOT\phpstorm\shell\open]

[HKEY_CLASSES_ROOT\phpstorm\shell\open\command]
@="\"C:\\Program Files (x86)\\JetBrains\\phpstorm.bat\" %1"

Custom bat file 'phpstorm.bat' pointing that one of PhpStorm:

Start /b "" "C:\Program Files (x86)\JetBrains\PhpStorm 2016.2.1\bin\PhpStorm.bat" %2 --line %3

dynasource · 2016-11-10T08:29:48Z

@dynasource I got it with a minimum of work.

This was time consuming:

trying to fix it with file:https:// and a chrome extension, but this seems to be restricted anno 2016
Fix the bat file to be headless
the line is not processed by PhpStorm 2016.2.2

dynasource · 2016-11-10T08:58:28Z

nice. You have to do --line before the file argument. Now it works fine!

dynasource · 2016-11-10T09:14:27Z

@thiagotalma, I tried your WScript. Its better because its headless and faster. Had to modify it to make it work ;) :

The x64 is reversed

bashkarev · 2016-11-10T10:12:29Z

Module.php

@@ -25,7 +25,7 @@
 */
 class Module extends \yii\base\Module implements BootstrapInterface
 {
- const TRACELINK_PHPSTORM_WINDOWS = '<a href="phpstorm:https:// {file} {line}">{file}:{line}</a>';
+ const TRACELINK_PHPSTORM = '<a href="phpstorm:https://open?url=file:https://{file}&line={line}">{file}:{line}</a>';


const TRACELINK_PHPSTORM = '<a href="phpstorm:https://open?url=file:https://{file}&line={line}">{text}</a>';

samdark · 2016-11-10T14:23:03Z

Module.php

@@ -25,6 +25,8 @@
 */
 class Module extends \yii\base\Module implements BootstrapInterface
 {
+ const TRACELINK_PHPSTORM = '<a href="phpstorm:https://open?url=file:https://{file}&line={line}">{text}</a>';


Why is it called PHPSTORM if the link is the same for all IDEs?

samdark · 2016-11-10T14:23:47Z

Panel.php

+ }
+ if ($this->module->traceLink === null) {
+ return $options['text'];
+ }else{


Formatting!

samdark · 2016-11-10T14:23:56Z

Panel.php

+ if ($this->module->traceLink === null) {
+ return $options['text'];
+ }else{
+ $options['file'] = str_replace('\\','/',$options['file']);


Formatting!

samdark · 2016-11-10T14:25:37Z

docs/guide/installation.md

+Wouldn't it be nice to be able to open files directly from the debug trace? 
+
+Well, you can!
+With a few settings and you're ready to go!


No need for and here.

samdark · 2016-11-10T14:28:17Z

tests/bootstrap.php

@@ -8,8 +8,13 @@
 $_SERVER['SCRIPT_NAME'] = '/' . __DIR__;
 $_SERVER['SCRIPT_FILENAME'] = __FILE__;

-require_once(__DIR__ . '/../vendor/autoload.php');
-require_once(__DIR__ . '/../vendor/yiisoft/yii2/Yii.php');
+if(is_dir(__DIR__ . '/../vendor/')){


Formatting!

Why is such condition needed?

please read the next lines

- renamed to traceLine - set traceLine to the CONST as default - fixed a false setting showing a textual line

dynasource · 2016-11-10T16:05:23Z

thanks to you all. Great work.

ilgiz-badamshin · 2016-11-29T15:24:45Z

Windows

What about instruction for unix systems?

cebe · 2016-11-29T18:07:12Z

@ilgiz-badamshin just found this: http:https://www.tuicool.com/articles/ANBBVj looks like it could be used as a basis for writing linux instructions in the docs. Would you be willing to write a guide and send a pull request?

cebe · 2016-11-29T20:09:43Z

created an issue for this: #172

cebe added the type:enhancement Enhancement label Nov 7, 2016

samdark approved these changes Nov 7, 2016

View reviewed changes

samdark requested changes Nov 7, 2016

View reviewed changes

thiagotalma added this to the 2.0.7 milestone Nov 7, 2016

thiagotalma assigned samdark Nov 7, 2016

Added ability to config the trace file and line number

7e0b2cd

thiagotalma changed the title ~~Show line number using the colon convention~~ Added ability to config the trace file and line number Nov 7, 2016

format raw

8d56549

dynasource reviewed Nov 7, 2016

View reviewed changes

Panel.php

/**

* @param array $trace The log trace

*

* @return string the trace line

Copy link

Member

dynasource Nov 7, 2016

Choose a reason for hiding this comment

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

missing @SInCE

dynasource reviewed Nov 7, 2016

View reviewed changes

dynasource suggested changes Nov 7, 2016

View reviewed changes

thiagotalma added 2 commits November 7, 2016 17:01

Adjustments required by staff

09d7a1d

revert blank spaces

1a2c5fd

dynasource previously approved these changes Nov 7, 2016

View reviewed changes

bashkarev reviewed Nov 7, 2016

View reviewed changes

add <a> tag

b803b9a

samdark reviewed Nov 7, 2016

View reviewed changes

thiagotalma added 2 commits November 7, 2016 18:29

traceLink with 'text' option

76e23d5

add 'text' placeholder in the phpdoc

07d6005

fix bootstrap

12a96f1

yiisoft locked and limited conversation to collaborators Nov 9, 2016

yiisoft unlocked this conversation Nov 9, 2016

added tests for new tracelink method

907b283

updated guide and updated the PHPSTORM logic

ff672e6

bashkarev reviewed Nov 10, 2016

View reviewed changes

to {text}

5ec712e

samdark reviewed Nov 10, 2016

View reviewed changes

dynasource added 3 commits November 10, 2016 16:24

- changed to a default IDE protocol in the CONST

e6500f9

- renamed to traceLine - set traceLine to the CONST as default - fixed a false setting showing a textual line

fix doc

606da5b

guide updates

71ce739

samdark approved these changes Nov 10, 2016

View reviewed changes

dynasource merged commit ed07955 into yiisoft:master Nov 10, 2016

thiagotalma deleted the lineNumber branch November 10, 2016 17:22

ilgiz-badamshin mentioned this pull request Nov 29, 2016

Fix dbPanel errors bedezign/yii2-audit#180

Merged

cebe mentioned this pull request Nov 29, 2016

Docs for linking stacktrace files to PHPStorm on Linux #172

Open

Added ability to config the trace file and line number #162

Added ability to config the trace file and line number #162

Conversation

thiagotalma commented Nov 7, 2016

dynasource commented Nov 7, 2016

bashkarev commented Nov 7, 2016 • edited Loading

samdark left a comment

Choose a reason for hiding this comment

thiagotalma commented Nov 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dynasource Nov 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bashkarev commented Nov 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dynasource left a comment

Choose a reason for hiding this comment

samdark commented Nov 7, 2016

bashkarev commented Nov 7, 2016 • edited Loading

thiagotalma commented Nov 7, 2016

dynasource left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bashkarev commented Nov 7, 2016

dynasource commented Nov 7, 2016

thiagotalma commented Nov 7, 2016

dynasource commented Nov 9, 2016

samdark commented Nov 9, 2016

thiagotalma commented Nov 9, 2016 • edited Loading

dynasource commented Nov 10, 2016 • edited Loading

dynasource commented Nov 10, 2016 • edited Loading

dynasource commented Nov 10, 2016

dynasource commented Nov 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dynasource commented Nov 10, 2016

ilgiz-badamshin commented Nov 29, 2016

cebe commented Nov 29, 2016

cebe commented Nov 29, 2016

bashkarev commented Nov 7, 2016 •

edited

Loading

dynasource Nov 7, 2016 •

edited

Loading

bashkarev commented Nov 7, 2016 •

edited

Loading

thiagotalma commented Nov 9, 2016 •

edited

Loading

dynasource commented Nov 10, 2016 •

edited

Loading

dynasource commented Nov 10, 2016 •

edited

Loading