changed "encodeUrl" in lib/pure/uri.nim. #7700
Conversation
|
Seems to make sense according to this discussion https://stackoverflow.com/questions/1634271/url-encoding-the-space-character-or-20 |
|
PHP has rawurlencode and rawurldecode according to rfc3986.
Maybe we should also add similar functions. |
|
@data-man encodeUrl(s: string, rawOn = false) |
|
Yes. |
|
And wget's code: url.c#L240 |
|
Python also uses For the best of both worlds this should indeed be customisable. But call the parameter |
Why? |
|
I vote for |
Two reasons:
|
Therefore, it is preferable to add two new procs, rather than add a new parameter. |
|
And I think that we need to add references to RFCs in the docs. |
I prefer a new parameter. |
|
@Araq |
|
How is this? |
|
@LeafChage proc encodeUrl*(s: string, rawOn = false): string =
## Encodes a value to be HTTP safe: This means that characters in the set
## ``{'A'..'Z', 'a'..'z', '0'..'9', '_'}`` are carried over to the result,
## a space is converted to ``'%20'`` and every other character is encoded as
## ``'%xx'`` where ``xx`` denotes its hexadecimal value.
result = newStringOfCap(s.len + s.len shr 2) # assume 12% non-alnum-chars
if rawOn:
for c in s:
case c
of 'a'..'z', 'A'..'Z', '0'..'9', '_', '-', '.', '~': add(result, c)
else:
add(result, '%')
add(result, toHex(ord(c), 2))
else:
for c in s:
case c
of 'a'..'z', 'A'..'Z', '0'..'9', '_': add(result, c)
of ' ': add(result, '+')
else:
add(result, '%')
add(result, toHex(ord(c), 2)) |
|
As I thought, I think so. |
|
And maybe to rename |
|
How did we end up here? I asked for |
|
Once again, revert back to the original. Then simply add a |
|
Maybe asRFC3986? |
|
If other characters need to be handled differently as well then please state what they are so we can decide what to do about them. |
|
I guess so. |
|
@dom96 |
|
I add only usePlus for now. |
|
Which is good? # A
proc encodeUrl*(s: string, usePlus = false): string =
result = newStringOfCap(s.len + s.len shr 2) # assume 12% non-alnum-chars
for i in 0..s.len-1:
case s[i]
of 'a'..'z', 'A'..'Z', '0'..'9', '_': add(result, s[i])
of ' ': add(result, if usePlus: "%20" else: "+")
else:
add(result, '%')
add(result, toHex(ord(s[i]), 2)) or # B
proc encodeUrl*(s: string, usePlus = false): string =
result = newStringOfCap(s.len + s.len shr 2) # assume 12% non-alnum-chars
for i in 0..s.len-1:
case s[i]
of 'a'..'z', 'A'..'Z', '0'..'9', '_': add(result, s[i])
of ' ':
if usePlus:
add(result, "%20")
else:
add(result, '+')
else:
add(result, '%')
add(result, toHex(ord(s[i]), 2)) I like the former. |
|
Well, the benchmarks: :)
|
|
Thank you:) |
|
I compare your latest version with #7700 (comment) |
|
I edited #7700 (comment) |
|
No. |
|
But why you left
Result №2:
|
|
Also, I continue to prefer on compliance with RFC3986. Results with dots differ!
|
Exactly, I think too that my code is not good. |
I prefer too. |
LeafChage
force-pushed
the
updateEncodeUrl
branch
from
5002375 to
e370990
Compare
|
|
How is this? |
|
I did some reading on this. Here is how I think this should work:
So this PR is almost ready. |
|
@dom96 |
|
I referred to escape.c. |
|
I ended up merging this manually and making some other changes too. Thank you for your PR :) |
I failed to access API with OAuth.
I want to "%20".