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

replace url\match in nginx.moon #498

Merged
merged 1 commit into from Dec 14, 2016

Conversation

Projects
None yet
3 participants
@mitnuh
Contributor

mitnuh commented Dec 14, 2016

Corrected fix for #490

@leafo

This comment has been minimized.

Show comment
Hide comment
@leafo

leafo Dec 14, 2016

Owner

Nice find, anchoring to the start with ^ should work as well.

Owner

leafo commented Dec 14, 2016

Nice find, anchoring to the start with ^ should work as well.

@leafo leafo merged commit ce92fb7 into leafo:master Dec 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -109,7 +109,8 @@ local ngx_req = {
end,
parsed_url = function(t)
local uri = ngx.var.request_uri
uri = uri:match("(.-)%?") or uri
local pos = uri:find("?")

This comment has been minimized.

@DarkWiiPlayer

DarkWiiPlayer Jul 24, 2017

Contributor

Shouldn't find("?", 1, true) be even faster?

@DarkWiiPlayer

DarkWiiPlayer Jul 24, 2017

Contributor

Shouldn't find("?", 1, true) be even faster?

This comment has been minimized.

@DarkWiiPlayer

DarkWiiPlayer Jul 24, 2017

Contributor

also those two lines could be turned into something like `local uri = uri:sub(1, (uri:find("?", 1, true) or 0) -1)

@DarkWiiPlayer

DarkWiiPlayer Jul 24, 2017

Contributor

also those two lines could be turned into something like `local uri = uri:sub(1, (uri:find("?", 1, true) or 0) -1)

This comment has been minimized.

@leafo

leafo Jul 24, 2017

Owner

probably an insignificant amount, there is no pattern on the single character pattern, so it will still end up matching it literally. Regarding combining the two lines, I don't really see an advantage to making it harder to read

@leafo

leafo Jul 24, 2017

Owner

probably an insignificant amount, there is no pattern on the single character pattern, so it will still end up matching it literally. Regarding combining the two lines, I don't really see an advantage to making it harder to read

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment