Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid regex is not shown to be invalid #93903

Closed
DasJott opened this issue Mar 31, 2020 · 8 comments
Closed

Invalid regex is not shown to be invalid #93903

DasJott opened this issue Mar 31, 2020 · 8 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug editor-find Editor find operations verified Verification succeeded
Milestone

Comments

@DasJott
Copy link

DasJott commented Mar 31, 2020

Issue Type: Bug

This regex: ([^?]+)\?([^:]+)\:(.*) is supposed to find lines like the following:
res = foo?bar:baz.
Instead it states: "No results"

I was told this exact same condition works under Windows. Both most recent versions, but different Electron versions. Maybe that's the thing?

regex tester

VS Code version: Code 1.43.2 (0ba0ca5, 2020-03-24T07:52:11.516Z)
OS version: Linux x64 5.4.27-1-MANJARO

System Info
Item Value
CPUs Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz (4 x 3500)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: disabled_software
skia_renderer: disabled_off_ok
video_decode: unavailable_off
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 1
Memory (System) 15.36GB (0.88GB free)
Process Argv
Screen Reader no
VM 0%
@jrieken
Copy link
Member

jrieken commented Mar 31, 2020

/needsMoreInfo

Is that inside the editor or global search?

@myfonj
Copy link

myfonj commented Mar 31, 2020

Backslash (\) before : should not be necessary in this context, and in this case removing it makes it work in text search. It seems like a discrepancy compared to standard RegExp we know from JavaScript, where backslash-escaping non-control characters usually makes no difference:

/a/.test('a') // true
/\a/.test('a') // true

In VSC text search it makes a difference:

image

image

Version: 1.43.2.
Commit: 0ba0ca52957102ca3527cf479571617f0de6ed50
Date: 2020-03-24T07:38:38.248Z
Electron: 7.1.11
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Windows_NT x64 10.0.16299

(I'm not sure but I think that regex engine used for document and file text search is not "just vanilla JS" (ripgrep perhaps?), and I'm not even sure if 100% compatibility with JS regex peculiarities is a goal. Nice bug anyway, good catch!)

@jrieken jrieken removed the info-needed Issue requires more information from poster label Mar 31, 2020
@DasJott
Copy link
Author

DasJott commented Mar 31, 2020

It is regarding text search within the editor. Didn't try the global one.
I also already tried with backslash removed but never got it. I sense that the problem is in the not operator like [^?]. Exchanging that to a simple .* worked as expected.

@myfonj
Copy link

myfonj commented Apr 1, 2020

I also already tried with backslash removed but never got it.
I sense that the problem is in the [negated character class] [^?]+. Exchanging that to a simple .* worked as expected.

So in your case just removing the extra backslash from \: (resulting in regexp ([^?]+)\?([^:]+):(.*)) yielded different outcome than expected? You had to further alter the negated character class or something?

It works for me, and I'd not expect this will be platform dependent:
image

@DasJott
Copy link
Author

DasJott commented Apr 1, 2020

Well, the same setting worked on Windows, but not in Linux.

@alexdima
Copy link
Member

alexdima commented Apr 1, 2020

The JS engine throws when constructing the regular expression:
image

@alexdima
Copy link
Member

alexdima commented Apr 1, 2020

Removing the unicode flags makes the regex valid: new RegExp('\\a', 'ig') works.

I think the validation code (that would display an error) forgets to use the unicode flag.

@alexdima alexdima added editor-find Editor find operations bug Issue identified by VS Code Team member as probable bug labels Apr 1, 2020
@alexdima alexdima changed the title regex search does not work properly Invalid regex is not shown to be invalid Apr 1, 2020
@alexdima alexdima added this to the March 2020 milestone Apr 1, 2020
@Tyriar Tyriar added the verified Verification succeeded label Apr 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug editor-find Editor find operations verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants
@myfonj @jrieken @Tyriar @alexdima @DasJott and others