Allow "name" argument on templates url_for()#2127
Allow "name" argument on templates url_for()#2127Kludex merged 2 commits intoKludex:masterfrom rosstitmarsh:master
Conversation
|
Hmm... Not sure if we should do this... The |
How do you mean? The |
aminalaee
left a comment
aminalaee
left a comment
There was a problem hiding this comment.
LGTM. I tested manually but maybe worth adding a simple test case.
|
At this point, why don’t we wait a bit and drop support for 3.7 first? Then we can make it a runtime enforced positional argument. |
But the context parameter shouldn't be... 🤔 |
|
Not sure what you mean. But also re-reading I realized we already merged a sibling PR, so no point in holding off. |
This was the test I made to make sure my change worked. I didn't add it to the PR because it seemed a bit trivial for something that will be redundant soon. I will add it if you want. def test_template_name_parameter(tmpdir, test_client_factory):
path = os.path.join(tmpdir, "index.html")
with open(path, "w") as file:
link = "<a href='{{ url_for('homepage', name=name) }}'>{{ name }}</a>"
file.write(f"<html>Hello, { link }</html>")
async def homepage(request):
return templates.TemplateResponse(
"index.html", {"request": request, "name": request.path_params["name"]}
)
app = Starlette(
debug=True,
routes=[Route("/{name}", endpoint=homepage)],
)
templates = Jinja2Templates(directory=str(tmpdir))
client = test_client_factory(app)
response = client.get("/world")
exptected_html = "<html>Hello, <a href='http://testserver/world'>world</a></html>"
assert response.text == exptected_html
assert response.template.name == "index.html"
assert set(response.context.keys()) == {"request", "name"} |
|
My point is... There's a parameter |
|
Ah, my bad. I was actually saying nonsense. |
4 participants
Summary
#2050 fixed the url_for functions not allowing "name" as a parameter, but it missed the templating url_for function
Checklist