search

mac-cain13 / xdebug-helper-for-chrome

Easily activate PHP debugging, profiling and tracing with this Xdebug Chrome extension
https://chrome.google.com/webstore/detail/eadndfjplgieldjbigjakmdgkmoaaaoc
307 stars 97 forks source link

trigger value support #57

Closed thornview closed 8 years ago

thornview commented 8 years ago

I had a situation where I needed a trace trigger value and noticed that your extension did not support that feature yet. So, I've added support for trace and profile trigger values into the code.

The trigger value is passed as the trace or profile cookie value, replacing the idekey you were using. If the user does not set a value, it defaults to "xdebug_eclipse", which seemed an innocuous value.

The config page got a bit full with the two extra boxes, so I modified the style a bit to keep it looking clean. I ended up with a save button that gets hidden on shorter screens, so it may need some tweaking.

Let me know if you run into any issues. I've been running it on my local box without any trouble but haven't had anyone else test it out.

mac-cain13 commented 8 years ago

Thanks for the PR! It looks good at first glance, I'd like to take it for a little test run before I merge and release it.

Will do that asap!

thornview commented 8 years ago

It's throwing an error that I haven't been able to track down yet. It doesn't seem to affect the actual performance of the extension, but I haven't been able to track down what's triggering the issue.

image

mac-cain13 commented 8 years ago

Hmm, looking at the documentation it seems that I never correctly covered the error case of sendMessage. The documentation states:

response: The JSON response object sent by the handler of the message. If an error occurs while connecting to the specified tab, the callback will be called with no arguments and runtime.lastError will be set to the error message. https://developer.chrome.com/extensions/tabs#method-sendMessage

So I think you'll get the undefined error when sendMessage fails, would be nice if we could fix that!

thornview commented 8 years ago

I was worried at first that my code was causing the error. I've been trying to find out what trips the sendMessage failure, but I haven't been able to create the issue on-demand. Other than wrapping it with try/catch, I'm not sure there's much to be done with it.

mac-cain13 commented 8 years ago

I think logging it or something like that would at least make it easier to debug then just having it run into a undefined. That could help find it if you test it for a little while maybe?!

thornview commented 8 years ago

Good point. I'll play around with it and see what I can find.

mac-cain13 commented 8 years ago

:+1:

thornview commented 8 years ago

I pushed an update that adds some error logging, and when trying to track down the bug I received the following error: Object {message: "Could not establish connection. Receiving end does not exist."}

The issue only happens in Opera (my primary browser); I haven't seen it in Chrome. Over the course of a day, Opera throws the error about three times, but it doesn't seem to effect the functionality of the extension. In other words, if I weren't logging the errors, I wouldn't know the extension had any problems.

I checked the original code (without my additions) and it throws the same error in Opera. So something in the existing code base is causing the problem.

Do you see this error in your testing? it doesn't seem to effect the performance of the extension, so I'm not sure it's a bug worth tracking down.

mac-cain13 commented 8 years ago

I've got reports of an error like the one your describing, but never seen it myself. Have to say that I don't debug that much PHP anymore.

Anyway, I agree that this issue shouldn't block merging this PR. I'll merge and release it asap, I'm currently a little busy, but should be able to do it this week. :)