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

Trying to Search using variable length look-behind suggests enabling search.usePCRE2, but no such setting #79109

Closed
ghost opened this issue Aug 14, 2019 · 13 comments · Fixed by microsoft/azuredatastudio#7206
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues verified Verification succeeded
Milestone

Comments

@ghost
Copy link

ghost commented Aug 14, 2019

  • VSCode Version: 1.37.0

Steps to Reproduce:

  1. ⇧⌘F (Open Find in Files)
  2. Enter a regex with a non-constant length look-behind, such as (?<!a.*)b
  3. Screenshot 2019-08-14 at 12 34 09
  4. Click Open Settings
  5. Screenshot 2019-08-14 at 12 50 04

Does this issue occur when all extensions are disabled?: Yes

@vscodebot
Copy link

vscodebot bot commented Aug 14, 2019

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@IllusionMH
Copy link
Contributor

/needsMoreInfo

This flag was deprecated in 1.37 because these features should be enabled automatically https://code.visualstudio.com/updates/v1_37#_search-regex-features

Do you see any errors in DevTools (Help > Toggle Developer Tools) when you performs search?

@vscodebot vscodebot bot added the info-needed Issue requires more information from poster label Aug 14, 2019
@vscodebot
Copy link

vscodebot bot commented Aug 14, 2019

Thanks for creating this issue! We figured it's missing some basic information or in some other way doesn't follow our issue reporting guidelines. Please take the time to review these and update the issue.

Happy Coding!

@ghost
Copy link
Author

ghost commented Aug 14, 2019

@IllusionMH No errors in the DevTools Console. I have also tried the same on the Insiders' build ba69d8dcccaebbfaa94906503728eb7c3e45b9c1 without plugins, can also reproduce there.

@ghost ghost changed the title Trying to Search using look-behind suggests enabling search.usePCRE2, but no such setting Trying to Search using variable length look-behind suggests enabling search.usePCRE2, but no such setting Aug 14, 2019
@ghost
Copy link
Author

ghost commented Aug 14, 2019

Realised that fixed-length look-behinds work, this only happens with variable-length ones.

Update: OK, it turns out PCRE (and most other regex libraries except .Net's) doesn't support infinite-length look-behind expressions...

@IllusionMH
Copy link
Contributor

Oh, looks like it is
/duplicate of #70156

@vscodebot vscodebot bot added the *duplicate Issue identified as a duplicate of another issue(s) label Aug 14, 2019
@vscodebot
Copy link

vscodebot bot commented Aug 14, 2019

Thanks for creating this issue! We figured it's covering the same as another one we already have. Thus, we closed this one as a duplicate. You can search for existing issues here. See also our issue reporting guidelines.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Aug 14, 2019
@IllusionMH
Copy link
Contributor

@peteVo do you see error message under the search field?
image

Probably I erroneously marked this as duplicate, because while inability to use variable length lookbehind can't be fixed, it make sense to use this issue to remove irrelevant error message in notification. @roblourens what do you think?

@roblourens
Copy link
Member

Yeah so when search returns a regex parse error, we suggest trying the PCRE2 setting because that supports some features that ripgrep by default doesn't. But, now that setting is deprecated because ripgrep is able to fall back on PCRE2 automatically. So this notification needs to be removed.

We could parse a better error message out of ripgrep's output but it's a bit of a pain and wouldn't be localized. e.g. in this case it looks like

regex could not be compiled with either the default regex engine or with PCRE2.

default regex engine error:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
regex parse error:
    (?<!a.*)b
    ^^^^
error: look-around, including look-ahead and look-behind, is not supported
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

PCRE2 regex engine error:
PCRE2: error compiling pattern at offset 0: lookbehind assertion is not fixed length

@roblourens roblourens self-assigned this Aug 14, 2019
@roblourens roblourens reopened this Aug 14, 2019
@roblourens roblourens added bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues and removed *duplicate Issue identified as a duplicate of another issue(s) info-needed Issue requires more information from poster labels Aug 14, 2019
@roblourens roblourens added this to the August 2019 milestone Aug 14, 2019
@IllusionMH
Copy link
Contributor

IllusionMH commented Aug 15, 2019

Are there a lot of features that are supported in JS (Find) but not in PCRE2 (Find in Files)?

If it's only one - won't it be enough to search for lookbehind assertion is not fixed length and have specific error for this case only because it will work fine in Find, but not in Find in Files?

UPD.

it's a bit of a pain and wouldn't be localized.

Looks like there is some mappings in rgErrorMsgForDisplay, but are no localization for regexp errors (in that function or in on UI, probably because some cases just capitalize first letter of Ripgrep's mesage):
image
image
I still see error messages in English even when set UI language to Russian.

@roblourens
Copy link
Member

Trying it, it seems like JS doesn't support variable length lookaround either, it just doesn't show an error message. I don't think there are a lot of features that fall into this gap but I don't have a list.

Regarding localization, I could map ripgrep errors to localized errors, but most regex parse errors will actually just come from JS because first we try to parse the regex in JS. I don't want to try to localize those. There are a lot and we will just make it harder to google.

@IllusionMH
Copy link
Contributor

Trying it, it seems like JS doesn't support variable length lookaround either, it just doesn't show an error message.

Strange. Just tried in VSCode DevTools, and it works just fine (in Chromium and same node version).

image

Am I doing something wrong?


As for localization - I saw cases where message returned almost as is from engine and understand why it makes a little sense to localize other.

will just make it harder to google.

Nice point!

@roblourens
Copy link
Member

Am I doing something wrong?

No, I am... I can't write regexes in the morning, you are correct.

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

Successfully merging a pull request may close this issue.

3 participants