hs.http must follow redirects #1889 #3279
Conversation
pdckxd
commented
Aug 22, 2022
pdckxd
commented
- new parameter "enableRedirect" for hs.http.doAsyncRequest
- add 5 tests to test this change
pdckxd
force-pushed
the
master
branch
3 times, most recently
from
0e7a1e5 to
8cd292e
Compare
extensions/http/libhttp.m
Outdated
|
|
||
| NSString* cachePolicy = nil; | ||
| if (lua_type(L, 6) == LUA_TSTRING) { | ||
| cachePolicy = [skin toNSObjectAtIndex:6]; | ||
| } | ||
|
|
||
| bool enableRedirect = true; | ||
| if (lua_type(L, 7) == LUA_TBOOLEAN) { |
There was a problem hiding this comment.
The way you have this now, to specify the enableRedirect option, the cachePolicy becomes non-optional. We are very inconsistent about this sort of thing across the codebase, but FWIW it's pretty easy to handle - add LS_TBOOLEAN as an option for the 6th argument and here check the lua_type() of position 6 first, to see if the boolean is there.
This scales very poorly with lots of additional optional arguments, but for the first one it's not too bad.
There was a problem hiding this comment.
Make sense. Let me revise. Thanks.
There was a problem hiding this comment.
Apart from optional string & boolean, I am thinking about if we can also support optional table for 6th parameter. Users can pass {cachePolicy= "xxx", enableRedirect=false} if they want to use both 2 parameters. What's your opinion? @cmsj
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1. new optional parameter "enableRedirect" for hs.http.doAsyncRequest 2. add 4 tests to test this change pr-feature
|
The optional table idea is what we should have done, but for now we're stuck with the idioms we have. |
