Debugging enhancments (custom Messages and more ) #150
Conversation
| @@ -61,7 +61,7 @@ module.exports = function (grunt) { | |||
| if (func) { | |||
| // FunctionExpression.BlockStatement.body(Array) | |||
| var funcBody = func.body.body; | |||
| esprima.parse('blocks.debug && blocks.debug.checkArgs(' + toValueString(data) + ', Array.prototype.slice.call(arguments), {})').body.forEach(function (chunk) { | |||
| esprima.parse('blocks.debug && blocks.debug.checkArgs(__DEBUG_METHOD, Array.prototype.slice.call(arguments), {}); var __DEBUG_METHOD = ' + toValueString(data) + ';').body.forEach(function (chunk) { | |||
There was a problem hiding this comment.
I am shooting in the dark here but if you put the var declaration in the start would you need to add it to .jshintrc?
Something like: var __DEBUG_METHOD = ' + toValueString(data) + ';' blocks.debug && blocks.debug.checkArgs(__DEBUG_METHOD, Array.prototype.slice.call(arguments), {});
There was a problem hiding this comment.
escodegen generates code like that from it.
I'm not sure why but I can look into that this evening.
Here is a build of this pr showing that the code is generated like you mentioned above.
There was a problem hiding this comment.
Ahh got it! We are unshifting the chunks into the function, which means we are adding the parsed body reversed. I'll add a reverse before the foreach so it's more easy to read.
Regarding the .jshintrc:
jshint doesn't even check the generated code, the error is shown (if _DEBUG_METHOD is removed when checking the source (e.g. src/mvc/Application.js).
We might could "fix" this when we add something macro-ish build step like
//@throwDebug('mesage', 'type')
// which would be translated to something
blocks.debug.throwMessage('message', __DEBUG_METHOD || null, 'type');I'm not sure if I like that more.
We could implemted as a regex & replace function on the emitted code or with travelling through the ast and search & replace the matching nodes.
I'm not sure which of the methods is easier and less error prone.
What do you think ?
There was a problem hiding this comment.
You could do:
This is pseudo code. I haven't tested it. I hope you get the idea.
func.body.body.unshift.apply(
func.body.body,
esprima.parse('var __DEBUG_METHOD = ' + toValueString(data) + ';' blocks.debug && blocks.debug.checkArgs(__DEBUG_METHOD, Array.prototype.slice.call(arguments), {});').body
);
Regarding .jshintc - I leaving this to you to decide.
P.S. When you are ready with the changes you could go ahead and merge this yourself.
There was a problem hiding this comment.
Yeah I got it.
I'll think about the jshint thing and merge it when I'm done.
|
I have added some comments in the code. Additionally, if you agree I want to give you push access to the repository. You can still work with pull requests to review our code. What do you think? |
|
I would love to get push access. |
|
Totally agree. I gave you access. Happy coding and thanks a lot for all the help. |
|
Awesome thanks! |
- Debug Build step now adds the method definition to a function-local-variable '__DEBUG_METHOD'. - Added __DEBUG_METHOD to the .jshintrc. - Added blocks.debug.throwMessage(Message: String, method: (e.g. __DEBUG_METHOD, level: [Notice, Warning, Error]) Level currently only prints an other message and changes the color. It's not perfect for example I don't like the __DEBUG_METHOD "global" in the jshintrc but it's the best and cleanest way I could come up with for now. Other approches involved scary Estree-Magic and wold fail in case of errors e.g. in user inserted function (callbacks). For now it should be enough.
[]-observable-reset and usage of not comment-ready data-queries in comments.
Views can not be constructed / added after the application has been started. Try to add your views synchronous before document.readyState is "complete".
debug-build-task to optimize readability.
Usage in a comment:
//@debugMessage('message', [Error|Warning|Info])
very basic manipalution of the estree but should do it for now.
Also removed the __DEBUG_METHOD again from the .jshintrc
The shopping example took about 3.8 seconds for the initial rendering due to garbe collection and allocation of our inlined '__DEBUG_METHOD' object. I speed it up at cost of more runtime memory (in the debug build) with moving all the inline definitions to blocks.debug.methods object. Every function now gets a unique id (just counting up). Now it's arround 1.5 seconds.
print something (not paused and not executing). This doesn't make the performance difference I hoped for because the optimnization is within a 15-20% mark and that's within the general js-engine noise. But it's at least fairly consistent and array.prototype.slice.apply(arguments) is also slow in some enigines. Optimizing the debug (and normal) build is on my private todo list.
|
I optimized the performance of the debug build (see the 2 last commits for details) and implemented the mentioned macro. The macro implementation is rather simple and not that flexible but we can change it later if decide we want to do more like that. Here's what the macro can:
The second argument of the macro can be omitted and normal quoted can be used instead if single quotes. I'll merge this now. Feel free to contact me if you have concerns (or questions) about something I merge (or commit to the master directly). |
Added a
blocks.debug.throwMessage(Message: String, [method:object], [level: String])function.I don't like that we need to insert the 'method' object manualy if we wan't it to print usage.
Therefore the debug grunt tasks now inlines the Method object to a local variable ``__DEBUG_METHOD`.
I also don't like that because I had to add it to the .jshintrc, but I think it should fit until i come up with a better way (unless you have a better idea).
Also implemented that for failing async view initialization:
Array-observables that get reset with a non-nullish and non-array type: closes #116
And the usage of non comment-able data-queries on comments:
Also if you ever tried to add a brakepint to an expression you know that it's not that easy...
Now it is (at least in the debug build):
That's a gif showing the initial expressions of the shopping example:
Edit: Forgot to mention above, but the
throwMessage-function will print a stack-trace if themethodargument isn't supplied. Should help in most cases to understand where a message might be comming from.