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

Update browser launch regex to support non-default logging frameworks #3842

Merged
merged 5 commits into from
Jun 11, 2020
Merged

Conversation

steveoh
Copy link
Contributor

@steveoh steveoh commented Jun 11, 2020

this regex only works with the default logging system. If you change the output template with something like serilog or another logging framework the regex will not match. Loosening it up to match this text will work in more cases.

this regex only works with the default logging system. If you change the output template with something like serilog or another logging framework the regex will not match. Loosening it up to match this text will work in more cases.
@gregg-miskelly
Copy link
Contributor

@steveoh You are just changing the documentation. Did you mean to change the default regex that the templates output also?

@steveoh
Copy link
Contributor Author

steveoh commented Jun 11, 2020

@gregg-miskelly i cant find the template file. So this is still only docs. Link me if you would like me to update it there also.

@steveoh steveoh requested a review from gregg-miskelly June 11, 2020 16:08
@gregg-miskelly
Copy link
Contributor

Here are the other spots:

C:\dd\omnisharp-vscode\package.json(2016):                "pattern": "^\"^\\\\\\\\s*Now listening on:\\\\\\\\s+(https?://\\\\\\\\S+)\""
C:\dd\omnisharp-vscode\src\assets.ts(303):        "pattern": "^\\\\s*Now listening on:\\\\s+(https?://\\\\S+)"

please dble check this one, there are too many escape characters so I just guessed.
@ghost
Copy link

ghost commented Jun 11, 2020

CLA assistant check
All CLA requirements met.

@steveoh
Copy link
Contributor Author

steveoh commented Jun 11, 2020

ok @gregg-miskelly please review those crazy escape sequence and let me know if i fouled it up.

@gregg-miskelly
Copy link
Contributor

I will give it a quick test, but I think you have that right.

@gregg-miskelly
Copy link
Contributor

Looks like you want to change the package.json line to :"\\\\bNow listening on:\\\\s+(https?://\\\\S+)".

@gregg-miskelly gregg-miskelly changed the title docs: remove starts with constraint Update browser launch regex to support non-default logging frameworks Jun 11, 2020
@steveoh
Copy link
Contributor Author

steveoh commented Jun 11, 2020

Ok, i made that change. Hopefully that's everything but if not I gave you access to edit my branch. Thanks!

@gregg-miskelly
Copy link
Contributor

Thanks for fixing this!

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.

2 participants