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

Correct regex completion #1950

Merged
merged 2 commits into from Sep 18, 2020
Merged

Correct regex completion #1950

merged 2 commits into from Sep 18, 2020

Conversation

333fred
Copy link
Contributor

@333fred 333fred commented Sep 18, 2020

Depending on whether the context is a verbatim string or not, the regex provider will escape the text, so we need to realize it ahead of time. Fixes #1949.

Depending on whether the context is a verbatim string or not, the regex provider will escape the text, so we need to realize it ahead of time. Fixes OmniSharp#1949.
Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ai, good catch

@333fred
Copy link
Contributor Author

333fred commented Sep 18, 2020

@filipw do you have any idea what could cause projects not be found on the Mac and Linux test legs? It seems that both the Mac and Linux MSBuild tests are seeing something like:

        [dbug]: OmniSharp.MSBuild.ProjectSystem
                Could not locate project for '/home/vsts/work/1/s/bin/Debug/OmniSharp.Roslyn.CSharp.Tests/net472/project.csproj'
        [dbug]: OmniSharp.MSBuild.ProjectSystem
                Could not locate project for 'dummy.csx'
        [dbug]: OmniSharp.Script.ScriptProjectSystem
                Could not locate project for 'dummy.csx'
        [trce]: OmniSharp.Roslyn.CSharp.Services.Completion.CompletionService
                Completions requested
        [trce]: OmniSharp.Roslyn.CSharp.Services.Completion.CompletionService
                Found 0 completions for dummy.csx:6,2

I'm especially confused because I'm doing nothing different in these tests than I'm doing for the other tests in the file, but these both are failing to locate projects.

@filipw
Copy link
Member

filipw commented Sep 18, 2020

yes I saw this before here 56afd27

at the time I just thought it was flaky and ended up removing it. Now it looks to me that CSX project system with project per CSX file somehow fails with the embedded language provider with Regex on *nix. I think it's OK to run these tests for *.cs only for now and we can investigate separately - at the end there is no regression here, it was just not being observed

@333fred
Copy link
Contributor Author

333fred commented Sep 18, 2020

I think it's OK to run these tests for *.cs only for now and we can investigate separately - at the end there is no regression here, it was just not being observed

Actually, these tests fail for .cs files as well.

@filipw
Copy link
Member

filipw commented Sep 18, 2020

hmm yes you are right. I would imagine we could repro this on a simple console project on Mono - perhaps it's a Roslyn bug?
either way, there is no regression here so adding [ConditionalFact(typeof(WindowsOnly))] would restrict it to Windows only for the time being. it shouldn't hold up this PR IMHO

@333fred
Copy link
Contributor Author

333fred commented Sep 18, 2020

Agreed, and done.

@filipw
Copy link
Member

filipw commented Sep 18, 2020

thanks a lot

@filipw filipw merged commit 4346937 into OmniSharp:master Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embedded regex language provider support does not seem to escape regex constructs correctly
2 participants