Updates per Adrian #2
Conversation
Address issue #1
HelloFlask/views.py
Outdated
| from re import match | ||
|
|
||
| from flask import Flask, render_template | ||
|
|
||
| from HelloFlask import app |
There was a problem hiding this comment.
I would go for hello_flask since snake_case names are more common for most python packages (some old stdlib packages aside, but unfortunately many older parts of stdlib aren't the best example for naming anyway)
There was a problem hiding this comment.
Yes, good point given that I made that piece a module and not just a folder name.
HelloFlask/views.py
Outdated
|
|
||
| @app.route('/hello/<name>') | ||
| def hello_there(name): | ||
| from datetime import datetime | ||
| now = datetime.now() | ||
|
|
||
| return render_template( | ||
| "hello_there.html", | ||
| title ='Hello, Flask', |
HelloFlask/views.py
Outdated
| return render_template( | ||
| "hello_there.html", | ||
| title ='Hello, Flask', | ||
| message = "Hello there, " + name + "!", | ||
| date = now.strftime("%A, %d %B, %Y at %X") | ||
| name = clean_name(name), |
HelloFlask/views.py
Outdated
| message = "Hello there, " + name + "!", | ||
| date = now.strftime("%A, %d %B, %Y at %X") | ||
| name = clean_name(name), | ||
| date = datetime.now() |
HelloFlask/views.py
Outdated
| from flask import Flask | ||
| from flask import render_template | ||
| from datetime import datetime | ||
| from re import match |
There was a problem hiding this comment.
usually people just import re and then use re.match() - this also avoids shadowing when people assign match = ... to the actual match object
There was a problem hiding this comment.
Makes sense...I'm still unclear when it's good to import a module as a whole vs. importing the part you need. :)
HelloFlask/views.py
Outdated
| return app.send_static_file('data.json') | ||
|
|
||
| def clean_name(name): |
There was a problem hiding this comment.
this is not really needed, since any variable passed to jinja is considered unsafe by default so any HTML there will be escaped
There was a problem hiding this comment.
Of course...I had it there in an intermediate step of the tutorial when I was just generating a plain text response, so I can omit it from the final version. Thanks for the reminder.
HelloFlask/views.py
Outdated
| # Filter the name argument to letters only using regular expressions. URL arguments | ||
| # can contain arbitrary text, so we restrict to safe characters only. | ||
| match_object = match("[a-zA-Z]+", name) |
There was a problem hiding this comment.
If you decide to keep it, I would replace this whole function with the much more simple:
return re.sub(r'[^a-zA-Z]+', '', name) or 'Friend'
the r prefix for the string is not needed here, but I think it's good practice to always use raw strings for regexps (so you don't forget it when adding something with a backslash later)
There was a problem hiding this comment.
I'll be omitting all that extra code anyway.
| <title>{{ title }}</title> | ||
| </head> | ||
| <body> | ||
| <span class="message">{{ message }}</span> It's {{ date }}. | ||
| <span class="message">Hello there, {{ name }}!</span> It's {{ date.strftime("%A, %d %B, %Y at %X") }}. | ||
| </body> | ||
| </html> |
There was a problem hiding this comment.
file doesn't have an ending linebreak. not pretty when you cat such a file in a terminal 😿
sounds like VS code actually has an option for this: microsoft/vscode#1666
dunno if it's enabled by default though (I hope it is)
There was a problem hiding this comment.
"files.insertFinalNewline": true is the setting, but it's false by default. :(
|
@ThiefMaster, take one more look. Renaming the folder means the diffs are gone, but the code is short and should have all the changes you requested. |
hello_app/views.py
Outdated
|
|
||
| @app.route('/') | ||
| def home(): | ||
| return render_template("home.html", title="Home") |
There was a problem hiding this comment.
I'd move that to the template using another {% block %}
There was a problem hiding this comment.
I don't understand what you mean. Is there a default rendering for / that we'd take advantage of here?
There was a problem hiding this comment.
I just meant the title="..." since that's something you'd usually put in your template.
The render_template calls you need to keep
|
Added the content block and revised the templates. Let me know if there's anything else, and thanks for your help! |
AzureRM Terraform Provider Contribution Guide
Templates naming
…t-lab Fix 101-devtest-labs automation test by adding some default value for variables
try microsoft#2 [skip ci]
Address issue #1