Draft for Issue 1418: Debug: extends setBreakpoint API to accept partial script name as a parameter

Draft for Issue 1418: Debug: extends setBreakpoint API to accept partial script name as a parameter

[Peter Rybin, 2011-06-03 23:19:40]

Hi Soren

I'm thinking about Issue 1418. This is my experimental implementation.

Could you please take a look and say what you think about this in general?
How do you feel about RegExp in protocol (depends on JavaScript RegExp
specification)?
How do you feel about RegExp as an engine? (Script matching against n
breakpoints will be bound to O(n) time).

My other idea is to deal with file path components. This would allow optimized
script->breakpoints matching if we need it, but someone would have to split
script name into a file path and I'm afraid embedder application would have to
do it (i.e. do "http://s/a/b/c.js?ul=en" -> ['a','b','c.js']).

Peter

[Søren Thygesen Gjesse, 2011-06-06 09:03:43]

I think using regexp is fine for this. If the frontend wants a simpler pattern
it can just convert it to a regexp.

You should add some tests and extend d8 to support it for debugging.

How about also adding break points of type sourceRegexp where a regexp match in
the actual source inserts a breakpoint? That might be useful for breaking in
code from eval.

http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js
File src/debug-debugger.js (right):

http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js#newcode72
src/debug-debugger.js:72: ScriptRegexp: 2 };
I guess that we do need an additional type here even thought he name matching
could be a regexp as well.

http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js#newcode1624
src/debug-debugger.js:1624: } else if (type == 'scriptRegexp') {
Can't we combine 'script' and 'scriptRegexp' here, and convert the name matching
to a regexp?

[Peter Rybin, 2011-06-16 11:02:12]

> I think using regexp is fine for this. If the frontend wants a simpler pattern
> it can just convert it to a regexp.

It's important to me to distinguish these two cases. Eclipse client has 2
different modes respectively.
Frontend converting back from regexp to string seems too much to me.

> How about also adding break points of type sourceRegexp where a regexp match
in
> the actual source inserts a breakpoint? That might be useful for breaking in
> code from eval.

I find it quite interesting. However I don't see a complete scenario right now.
E.g. who is going to build regexp? Do we want to tie breakpoint line number to
the matching line in source? Does it include cases when eval sources have
variable parts?

(inline comments come in next message)

[Peter Rybin, 2011-06-16 11:02:20]

http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js
File src/debug-debugger.js (right):

http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js#newcode72
src/debug-debugger.js:72: ScriptRegexp: 2 };
On 2011/06/06 09:03:43, Søren Gjesse wrote:
> I guess that we do need an additional type here even thought he name matching
> could be a regexp as well.

(I take it as "we do NOT need")

Actually on the client I'd like to distinguish these two type (I work with them
differently). Since V8 lists breakpoints back to client ("listbreakpoints"
request), this information should be somehow kept.

But we can drop new ScriptBreakPointType and create polymorphic
ScriptNameMatcher that will handle all this.

http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js#newcode1624
src/debug-debugger.js:1624: } else if (type == 'scriptRegexp') {
On 2011/06/06 09:03:43, Søren Gjesse wrote:
> Can't we combine 'script' and 'scriptRegexp' here, and convert the name
matching
> to a regexp?

I'd like to keep exact name and regexp separate. It corresponds to two different
modes in my client. Of course I can analyze regexp source and detect that the
pattern is exact string, but I think it's too tricky.

[Søren Thygesen Gjesse, 2011-06-16 11:53:30]

On 2011/06/16 11:02:20, Peter Rybin wrote:
> http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js
> File src/debug-debugger.js (right):
> 
> http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js#newcode72
> src/debug-debugger.js:72: ScriptRegexp: 2 };
> On 2011/06/06 09:03:43, Søren Gjesse wrote:
> > I guess that we do need an additional type here even thought he name
matching
> > could be a regexp as well.
> 
> (I take it as "we do NOT need")
> 
> Actually on the client I'd like to distinguish these two type (I work with
them
> differently). Since V8 lists breakpoints back to client ("listbreakpoints"
> request), this information should be somehow kept.
> 
> But we can drop new ScriptBreakPointType and create polymorphic
> ScriptNameMatcher that will handle all this.
> 
>
http://codereview.chromium.org/7091001/diff/1/src/debug-debugger.js#newcode1624
> src/debug-debugger.js:1624: } else if (type == 'scriptRegexp') {
> On 2011/06/06 09:03:43, Søren Gjesse wrote:
> > Can't we combine 'script' and 'scriptRegexp' here, and convert the name
> matching
> > to a regexp?
> 
> I'd like to keep exact name and regexp separate. It corresponds to two
different
> modes in my client. Of course I can analyze regexp source and detect that the
> pattern is exact string, but I think it's too tricky.

It is OK to keep the two types distinct.