added pydevd tag

[stefanklug]

Hi,

your template debug tags are great! I added a tag "pydevd" to jump into the pydev debugger. I didn't add it to the set_trace tag, to keep the pdb functionality, even if pydevd is available. Would be great if you could merge it.

Regards Stefan

[calebsmith]

Hi Stefan,

First, thanks for the contribution! I have a couple of thoughts with regard to the code changes, but I think this is a really good start.

I've never used pydevd, but I'm looking forward to trying it out later. As a result, it may be a few days before I can really fully evaluate this pull request, but the code seems correct on a basic level.

1. I'm wondering if just the imports can be in the try block, and the variable assignments and set_trace() can be in an else block or just unindented after the try/except blocks? It isn't a big deal but it's good to move any code out of try if possible in my opinion. As an example I mean:

try:
    import socket
    import pydevd
Except(ImportError, socket.error):
    pydevd_not_available = True
availables = ....

1. The only other thing is, and I appreciate that you've followed the existing style and used the require_template_debug decorator, that maybe some of this code needs to be another function. In particular, this part:

availables = get_variables(context)
for var in availables:
    locals()[var] = context[var]

Of course this is largely a copy/paste from set_trace(), and they're both doing the correct thing. However, I can see that more debug tools might be added eventually, and it will be nice to have one implementation for this.

What I'm suggesting is some light refactoring in which this part that crams the context into the local scope lives in its own function. Something like:

def set_context_in_locals(context):
    availables = get_variables(context)
    for var in availables:
        locals()[var] = context[var]

Let me know any thoughts on the above and thanks again. Looking forward to trying this out soon.

[stefanklug]

I refactored the parts you mentioned. Now I need two try/catch block, because the Exceptions are thrown at different places. But the code is cleaner. I left the code duplication with availables... in there for now. The locals()[..] thing is not officially supported ( see http://stackoverflow.com/questions/8028708/dynamically-set-local-variable-in-python ) and is most likely not portable. (It works nicely for me) I thought about dynamically creating a function with all the locals as named arguments and inside that function one could call the settrace(). But thats also quite hackish.

[calebsmith]

Thanks for the clean up and I like the latest changes.

You are absolutely right about the locals()[..] hack. I was okay with including that because I think the value of the feature in this case outweighs the code smell, especially because none of the tags are intended to run in a production environment. I want to have the context variables available in the local scope lest I have to use context['varname'].attr, which gets old quickly.

It is still nasty, but I like the idea of creating and calling a function with named parameters a bit better. I'll make that a completely separate change though and just pull this in whenever I get a chance to set up pydevd and try everything out.

Thanks again for the clean up and ideas and I'm looking forward to trying it out.

[calebsmith]

Hey, thanks again for the contribution.