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

Use IndexOfAnyInRange in StripBidiControlCharacters #78658

Merged
merged 2 commits into from Nov 22, 2022

Conversation

MihaZupan
Copy link
Member

This method is called on the host when creating a Uri.
The 99.9% case is that there will be no such characters, so we can let IndexOfAnyInRange quickly scan through the input.

Saw this while looking for cases where we can use IndexOfAnyValues (#78204).

@MihaZupan MihaZupan added this to the 8.0.0 milestone Nov 21, 2022
@MihaZupan MihaZupan requested a review from a team November 21, 2022 21:38
@MihaZupan MihaZupan marked this pull request as ready for review November 21, 2022 21:38
@ghost ghost assigned MihaZupan Nov 21, 2022
@ghost
Copy link

ghost commented Nov 21, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

This method is called on the host when creating a Uri.
The 99.9% case is that there will be no such characters, so we can let IndexOfAnyInRange quickly scan through the input.

Saw this while looking for cases where we can use IndexOfAnyValues (#78204).

Author: MihaZupan
Assignees: -
Labels:

area-System.Net

Milestone: 8.0.0

@stephentoub stephentoub merged commit ae70706 into dotnet:main Nov 22, 2022
@danmoseley
Copy link
Member

danmoseley commented Nov 22, 2022

Do you plan to look for opportunities in aspnetcore? If not it might be worth opening a suggestion for someone to do that.

@stephentoub
Copy link
Member

There are multiple places aspnetcore can/should end up using the new API, e.g. dotnet/aspnetcore#33776 (comment)

@MihaZupan
Copy link
Member Author

@gfoidl had a PR in aspnetcore (dotnet/aspnetcore#44041) that was closed in favour of waiting for these APIs.

I can take a look at those cases as well if he doesn't beat me to it, but I imagine it's gonna take a little bit for the runtime changes to flow into that repo anyway.

@danmoseley
Copy link
Member

@gfoidl do you plan to have a run at aspnetcore cases now this marvelous API is available?

@gfoidl
Copy link
Member

gfoidl commented Nov 22, 2022

Yep, that's on my todo-list for the next days (when the APIs is "flowed" to aspnet).

@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants