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

Find with Regex Lookbehind #100569

Closed
merkuriy opened this issue Jun 19, 2020 · 11 comments
Closed

Find with Regex Lookbehind #100569

merkuriy opened this issue Jun 19, 2020 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues verified Verification succeeded
Milestone

Comments

@merkuriy
Copy link

merkuriy commented Jun 19, 2020

Issue Type: Bug

  1. Regex for find:
(?<=\n)\w

Results:

  • Find (by currnet file editor) 🠊 normal.
  • Find in Files 🠊 Regexp parse error: lookbehind assertion is not fixed length

  1. Regex for find:
(?<=\s)\w

Results:

  • Find (by currnet file editor) 🠊 normal.
  • Find in Files 🠊 normal.

VS Code version: Code 1.46.0 (a5d1cc2, 2020-06-10T09:03:20.462Z)
OS version: Windows_NT x64 10.0.19041
Remote OS version: Linux x64 4.19.84-microsoft-standard

System Info
Item Value
CPUs Intel(R) Core(TM) i5-4200H CPU @ 2.80GHz (4 x 2794)
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: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 7.92GB (1.35GB free)
Process Argv
Screen Reader no
VM 0%
Item Value
Remote WSL: ubuntu-19-04-server
OS Linux x64 4.19.84-microsoft-standard
CPUs Intel(R) Core(TM) i5-4200H CPU @ 2.80GHz (4 x 2793)
Memory (System) 6.14GB (4.78GB free)
VM 0%
Extensions (23)
Extension Author (truncated) Version
Angular-BeastCode Mik 8.1.2
vscode-apache mrm 1.2.0
remote-wsl ms- 0.44.3
vscode-nginx wil 0.7.2
commit-message-editor ada 0.13.0
vscode-intelephense-client bme 1.4.1
gitignore cod 0.6.0
vscode-eslint dba 2.1.5
githistory don 0.6.5
vscode-powertools ego 0.60.1
apacheconf-snippets eim 1.3.0
php-cs-fixer jun 0.1.154
vscode-phpfmt kok 1.0.30
sshextension kon 0.5.0
vscode-docker ms- 1.3.1
sqltools mtx 0.22.7
sqltools-driver-pg mtx 0.0.5
vscode-yaml red 0.8.0
format-html-in-php rif 1.6.2
vscode-scss-formatter sib 2.0.1
sass-indented syl 1.8.6
language-stylus sys 1.11.0
markdown-all-in-one yzh 3.0.0
@merkuriy merkuriy changed the title Regex lookbehind error Find with Regex Lookbehind Jun 19, 2020
@roblourens roblourens added bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues labels Jun 19, 2020
@roblourens roblourens added this to the Backlog milestone Jun 19, 2020
@roblourens
Copy link
Member

This is because I try to normalize the newline to \r?\n

@rebornix rebornix removed their assignment Jun 24, 2020
@ArturoDent
Copy link

ArturoDent commented Jun 26, 2020

I "stumbled" on this as a workaround in a search across files regex:

(?<=[\n])\w

Making the \n into a character class of its own seems to work. At least on Windows10. Test on your OS.

Perhaps @roblourens can explain why. Does the character class version not get expanded to \r?\n.? Will [\r\n] work in more situations?

@roblourens
Copy link
Member

It does get expanded but in that case, \r is also inside the character class and the whole thing matches a single character, thus is still "fixed length"

@fw623
Copy link

fw623 commented Aug 26, 2020

A possible solution that comes to mind, is to instead of directly expanding \n in a lookbehind, expand it to ((LOOKBEHIND_WITH_R)(...))|((LOOKBEHIND_WITHOUT_R)(...))

This would probably be terrible for performance, but maybe better than having to use [\n] in lookbehind (and actually being unable to search exactly only for \n).

Example

Instead of (?<=\n)\w(?<=\r?\n)\w,
do (?<=\n)\w((?<=\r\n)\w)|((?<=\n)\w).

@connor4312 connor4312 modified the milestones: Backlog, November 2020 Nov 13, 2020
@connor4312 connor4312 self-assigned this Nov 13, 2020
@connor4312
Copy link
Member

We will no longer normalize CRLF in lookbehinds. And will do so more intelligently in other cases.

@ghost
Copy link

ghost commented Nov 15, 2020

Issue Type: Bug

  1. Regex for find:
(?<=\n)\w

Results:

  • Find (by currnet file editor) 🠊 normal.

  • Find in Files 🠊 Regexp parse error: lookbehind assertion is not fixed length


  1. Regex for find:
(?<=\s)\w

Results:

  • Find (by currnet file editor) 🠊 normal.

  • Find in Files 🠊 normal.


VS Code version: Code 1.46.0 (a5d1cc2, 2020-06-10T09:03:20.462Z)

OS version: Windows_NT x64 10.0.19041

Remote OS version: Linux x64 4.19.84-microsoft-standard

System Info

|Item|Value|

|---|---|

|CPUs|Intel(R) Core(TM) i5-4200H CPU @ 2.80GHz (4 x 2794)|

|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: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: enabled
webgl2: enabled|

|Load (avg)|undefined|

|Memory (System)|7.92GB (1.35GB free)|

|Process Argv||

|Screen Reader|no|

|VM|0%|

|Item|Value|

|---|---|

|Remote|WSL: ubuntu-19-04-server|

|OS|Linux x64 4.19.84-microsoft-standard|

|CPUs|Intel(R) Core(TM) i5-4200H CPU @ 2.80GHz (4 x 2793)|

|Memory (System)|6.14GB (4.78GB free)|

|VM|0%|

Extensions (23)

Extension|Author (truncated)|Version

---|---|---

Angular-BeastCode|Mik|8.1.2

vscode-apache|mrm|1.2.0

remote-wsl|ms-|0.44.3

vscode-nginx|wil|0.7.2

commit-message-editor|ada|0.13.0

vscode-intelephense-client|bme|1.4.1

gitignore|cod|0.6.0

vscode-eslint|dba|2.1.5

githistory|don|0.6.5

vscode-powertools|ego|0.60.1

apacheconf-snippets|eim|1.3.0

php-cs-fixer|jun|0.1.154

vscode-phpfmt|kok|1.0.30

sshextension|kon|0.5.0

vscode-docker|ms-|1.3.1

sqltools|mtx|0.22.7

sqltools-driver-pg|mtx|0.0.5

vscode-yaml|red|0.8.0

format-html-in-php|rif|1.6.2

vscode-scss-formatter|sib|2.0.1

sass-indented|syl|1.8.6

language-stylus|sys|1.11.0

markdown-all-in-one|yzh|3.0.0

@ghost
Copy link

ghost commented Nov 15, 2020

@mjbvz mjbvz added the verified Verification succeeded label Dec 4, 2020
@mjbvz
Copy link
Collaborator

mjbvz commented Dec 4, 2020

@roblourens I verified this search no longer triggers an error. However on windows I only see a workspace search for (?<=\n)\w returning results from open files. This may be expected but it seemed odd

@mjbvz
Copy link
Collaborator

mjbvz commented Dec 4, 2020

Actually nevermind, I still see errors in the developer tools:

Repro

  1. On windows in the VS Code Codebase
  2. Do a workspace search for (?<=\n)\w
  3. Open Developer tools

I see lots of errors that look like:

[IPC Library: Search] Uncaught Exception:  TypeError: Cannot read property 'startLineNumber' of undefined
    at c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:172:869
    at new t.TextSearchMatch (c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:172:966)
    at Object.t.createTextSearchResult (c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:183:949)
    at y.createTextSearchMatch (c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:210:624)
    at y.handleLine (c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:209:618)
    at y.handleDecodedData (c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:209:312)
    at y.handleData (c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:209:133)
    at Socket.<anonymous> (c:\Users\matb\AppData\Local\Programs\Microsoft VS Code Insiders\resources\app\out\vs\workbench\services\search\node\searchApp.js:208:77)
    at Socket.emit (events.js:228:7)
    at addChunk (_stream_readable.js:309:12)
    at readableAddChunk (_stream_readable.js:290:11)
    at Socket.Readable.push (_stream_readable.js:224:10)
    at Pipe.onStreamRead (internal/stream_base_commons.js:181:23)
t.log @ workbench.desktop.main.js:sourcemap:310

There errors are not shown to the user so I was not aware they were happening until checking the dev tools

@mjbvz mjbvz reopened this Dec 4, 2020
@mjbvz mjbvz added verification-found Issue verification failed and removed verified Verification succeeded labels Dec 4, 2020
@connor4312
Copy link
Member

connor4312 commented Dec 4, 2020

That looks like a quirk in ripgrep, they return results without any submatches. This would have had the same error before my changes (if it were possible to execute the search) and I'm sure there are other times people ran into this. It seems like a questionable bug in ripgrep. I put in a small patch to have the range behavior be the same as what the file matcher does, just showing the first character as the selection

image

cc @roblourens

@connor4312 connor4312 removed the verification-found Issue verification failed label Dec 4, 2020
@roblourens
Copy link
Member

That upstream issue is BurntSushi/ripgrep#1412, thought it was handled, maybe it got lost. Thanks for the fix @connor4312

@roblourens roblourens added the verified Verification succeeded label Dec 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jan 19, 2021
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 search Search widget and operation issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants
@roblourens @merkuriy @rebornix @connor4312 @mjbvz @fw623 @ArturoDent and others