Handle sites that use absolute links and sites that require the final slash in the URL #121
Conversation
Some web sites will return 404 if you fetch a directory without the final slash. For example, https://archive.mozilla.org/pub/ works, https://archive.mozilla.org/pub does not. We need to do two things to accommodate this: * When processing the root URL of the filesystem, instead of stripping off the final slash, just set the offset to ignore it. * In the link structure, store the actual URL tail of the link separately from its name, final slash and all if there is one, and append that instead of the name when constructing the URL for curl.
|
lol I see somebody else already encountered this #115. I guess I should have checked for PRs first. Anyway I'll leave it to you to decide which of these PRs to prefer. ;-) |
|
I will say that I think my version is more robust and better documented. |
On some sites, the link to each subfolder is an absolute link rather than a relative one. To accommodate this, convert the links from absolute to relative before storing them in the link table.
I believe an appropriate expectation is that if the user enables debugging with a command-line flag, then that should also enable messagse designated as debug messages in the code to be printed.
Some sites put unencoded characters in their href attributes that really should be encoded, most notably spaces. Curl won't accept a URL with a space in it, and perhaps other such characters as well. Address this by properly encoding characters in URLs before feeding them to Curl.
|
OK, there's another bug fix for you in the last commit, plus a couple ancillary commits that aren't strictly speaking necessary for the bug fixes but I think are a good idea. 🤷 |
|
I would suggest to open separate merge requests for the different proposed changes. This PR is getting a little out of hand. |
|
I am happy to do that if someone is actually going to look at the changes but considering that I opened the PR weeks ago and there has been no response I don't know if that is going to happen. |
|
Also, most of the changes in the PR before the autotools changes I pushed today are intended for essentially the same purpose, i.e., getting this tool to work on sites where it doesn't currently, and therefore I think it's reasonable to review them together. |
Sorry I had been a bit busy with my personal life. I just hadn't got time to look at it. |
|
@jikamens, I agree with what @jcharaoui said. Also, you seem to be very good at software engineering. I would be super grateful if you write some sort of test system for this. If you write a test system to prove that all your changes work, I will approve your pull request. Right now I just have to manually test things. This really isn't ideal. I started this project before I became a professional software engineer. This sort of testing wouldn't fly in my company... I am happy to approve your autotool related changes, if they were in a different pull request. |
install-sh
Outdated
| #!/bin/sh | ||
| # install - install a program, script, or datafile | ||
|
|
||
| scriptversion=2020-11-14.01; # UTC |
There was a problem hiding this comment.
Where is this from? Why is the scriptversion nearly 3 years old?
missing
Outdated
| @@ -0,0 +1,207 @@ | |||
| #! /bin/sh | |||
| # Common wrapper for a few potentially missing GNU programs. | |||
|
Hi @fangfufu, A few things:
|
|
@jikamens , I am interested in the autotool changes. Please submit a separate commit for it. |
I didn't imply in any way that you have to create unit-tests to get these accepted. I was just saying that it would be a good contribution to do. I might just create an issue for it. |
Autotool changes can be done simultaneously as your other bug fixes. Once again, thanks for your contribution. I think autotool changes would be good for this project. |
|
See #123. |
No description provided.