1005469 - Using :# as a search query causes "TypeError: query is undefined searchcursor.js:27"

Status: RESOLVED INVALID

(DevTools :: Debugger, defect, P3)

Description

[Adrian Aichner [:anaran]]

Most invalid characters are converted to 1 and line one is highlighted.

e.g.
:- => :1
:a => :1
:@ => :1
:: => :1
:* => :1

SFSG

However
-------

:# => :#
does nothing (previously selected line stay selected).

Furthermore
-----------

:3:4:5:6 => :3:4:5:6
and selects line 6.

Is this a feature to document a sequence of lines of interest?
I almost like this already.

Finally
-------

if the line sequence syntax is intended, then two components would surely look like
:LINE:CHARACTER

Wouldn't it be better to only support
:LINE
:LINE:CHARACTER
and correct all others to
:1
instead?

:LINE:CHARACTER would also be a handly feature for minimize scripts and for Jump to line in Scratchpad debugging (even though that is a separate implementation the established Error.stack syntax file:LINE:CHARACTER seems like a good standard).

Comment 1

[Benjamin Kerensa [:bkerensa]]

Rob,

Is this behavior intended?

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

The behavior is a combination of error handling and bugs.

(In reply to adrian from comment #0)
> Most invalid characters are converted to 1 and line one is highlighted.
> 
> e.g.
> :- => :1
> :a => :1
> :@ => :1
> :: => :1
> :* => :1
> 
> SFSG

Working error handling.

> However
> -------
> 
> :# => :#
> does nothing (previously selected line stay selected).

Broken error handling. This results in:

TypeError: query is undefined searchcursor.js:27

This is also a case where two different query tokens are combined in a potentially meaningful, but rarely useful way (e.g. ":5#var" -> find the string "var" at line 5). Currently we don't do a very good job of handling corner cases like this.

> Furthermore
> -----------
> 
> :3:4:5:6 => :3:4:5:6
> and selects line 6.
> 
> Is this a feature to document a sequence of lines of interest?
> I almost like this already.

Working error handling (per Postel's principle!). Only the last query term is used, so it wouldn't be useful for marking a sequence of lines (no highlighting happens for the other terms).

> Finally
> -------
> 
> if the line sequence syntax is intended, then two components would surely
> look like
> :LINE:CHARACTER
> 
> Wouldn't it be better to only support
> :LINE
> :LINE:CHARACTER
> and correct all others to
> :1
> instead?
> 
> :LINE:CHARACTER would also be a handly feature for minimize scripts and for
> Jump to line in Scratchpad debugging (even though that is a separate
> implementation the established Error.stack syntax file:LINE:CHARACTER seems
> like a good standard).

I agree that having a ":line:column" search term could be useful when we add support for setting column breakpoints in the UI. In the meantime I would prefer it if we used this bug for the bug mentioned above.

Comment 3

[Adrian Aichner [:anaran]]

To make all this actionable I am going to suggest the following:

(In reply to Panos Astithas [:past] (overloaded, please needinfo) from comment #2)
> The behavior is a combination of error handling and bugs.
> 
...
> > :# => :#
> > does nothing (previously selected line stay selected).
> 
> Broken error handling. This results in:
> 
> TypeError: query is undefined searchcursor.js:27
> 
> This is also a case where two different query tokens are combined in a
> potentially meaningful, but rarely useful way (e.g. ":5#var" -> find the
> string "var" at line 5). Currently we don't do a very good job of handling
> corner cases like this.

I suggest to do this:

:# => :1

Makes clear # is not supported.

> 
> > Furthermore
> > -----------
> > 
> > :3:4:5:6 => :3:4:5:6
> > and selects line 6.
> > 
> > Is this a feature to document a sequence of lines of interest?
> > I almost like this already.
> 
> Working error handling (per Postel's principle!). Only the last query term
> is used, so it wouldn't be useful for marking a sequence of lines (no
> highlighting happens for the other terms).

I suggest to do this:

:3:4:5:6 => :6

Makes clear sequences are not supported.

Leaving :3:4:5:6 in the query field would be surprising given the corrections made the other cases above.

> 
> > Finally
> > -------
> > 
> > if the line sequence syntax is intended, then two components would surely
> > look like
> > :LINE:CHARACTER
> > 
> > Wouldn't it be better to only support
> > :LINE
> > :LINE:CHARACTER
> > and correct all others to
> > :1
> > instead?
> > 
> > :LINE:CHARACTER would also be a handly feature for minimize scripts and for
> > Jump to line in Scratchpad debugging (even though that is a separate
> > implementation the established Error.stack syntax file:LINE:CHARACTER seems
> > like a good standard).
> 
> I agree that having a ":line:column" search term could be useful when we add
> support for setting column breakpoints in the UI. In the meantime I would
> prefer it if we used this bug for the bug mentioned above.


I suggest to do this:

:LINE:CHARACTER => :CHARACTER

Makes clear :LINE:CHARACTER query is not supported and is consistent with the correction of sequence above.

Leaving :LINE:CHARACTER in the query field would be surprising given the corrections made the other cases above.

Addressing only the :# case would still leave inconsistencies.

There is substantial work involved in getting a bug fixed, tested, reviewed.

I wouldn't want to go through this loop for each case individually, leaving partial breakage along the way.

Comment 4

[Adrian Aichner [:anaran]]

(In reply to adrian.aichner from comment #3)
> There is substantial work involved in getting a bug fixed, tested, reviewed.

Let me reword that to...

There is substantial work involved in getting a bug fix implemented, tested, reviewed, tried, checked in.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

I'm OK with that.

Comment 6

[Jason Laster [:jlast]]

closing as this is for the old debugger.