feat: takes into account when statics not in host_url root #22
Conversation
|
Hi, Thanks, I haven't gotten a chance to test it locally but does this work when |
I just wanted to test this separately. Also, It seemed to me that I should not mix |
|
Will have to think about that one a bit. Just replying to acknowledge your response. Since it's the work week now I'm a little slammed, but I want to let you know this will for sure get merged in 1 way or another. |
|
@nickjj ¿any progress with this? |
| def parse_digest_host_url(host_url_prefix): | ||
| """ | ||
| Detect if host_url_prefix contains a path element and returns a tuple with | ||
| the elements (host_part, path_part), for example for |
There was a problem hiding this comment.
Can you re-word this line to be:
the elements (host_part, path_part), for example:
|
|
||
| host_url_prefix = "https://cdn.example.com/myapp/some/path/" | ||
|
|
||
| This function should return: |
There was a problem hiding this comment.
Can you change this line to:
This function will return results such as:
| host_url_prefix = "https://cdn.example.com/myapp/some/path/" | ||
|
|
||
| This function should return: | ||
| ('https://cdn.example.com', "/myapp/some/path/") |
There was a problem hiding this comment.
Can you change this line into these lines:
# When there's no path:
("https://cdn.example.com")
# When there's a path:
("https://cdn.example.com", "/my/custom/path/")
| @@ -84,3 +86,28 @@ def static_url_for(self, endpoint, **values): | |||
| merged_values.get("filename")) | |||
| else: | |||
| return flask_url_for(endpoint, **merged_values) | |||
|
|
|||
|
|
|||
| def parse_digest_host_url(host_url_prefix): | |||
There was a problem hiding this comment.
Can you rename this function to _parse_digest_host_url since it's private?
| @@ -46,8 +46,10 @@ def _load_manifest(self, app): | |||
| return manifest_dict | |||
|
|
|||
| def _prepend_host_url(self, host, filename): | |||
| return urljoin(self.host_url, | |||
| "/".join([self.static_url_path, filename])) | |||
| parts = list( # removing empty strings | |||
There was a problem hiding this comment.
Any chance we can write rewrite this and the line below it to be a more clear on what it does?
| @@ -0,0 +1,39 @@ | |||
| from flask_static_digest import parse_digest_host_url | |||
There was a problem hiding this comment.
Thoughts on replicating all of these tests without using pytest, following the same patterns used in the existing test suite?
|
I'm beginning to think this PR along with That completely negates needing What do you think? |
It depends on if it works, have you tested ? i don't have time right now, if u think that work ok, u should update the README.md and close this PR. |
|
I haven't tested it yet but I will in the next couple of days. If it works I'll update the readme and ship a new version with that option removed along with adding new docs to go over how to do the same thing with Flask's built in |
|
Nope, it doesn't work. Which means the option and this PR is still going to be a good idea. |
|
This has been resolved in: #38 |
I added some tests with
pytest, think I included all the cases, let me know if i need to include more examplesCloses #21