Fix the wget footer #1043

Yusuto · 2023-02-20T15:17:45Z

...by having it use the absolute url instead of the breadcrumbs.
I think the changes should be rather self-explanatory, I tested my changes before opening this PR and also ran Clippy and rustfmt and neither said anything, but please still check what I did, just in case.

src/renderer.rs

svenstaro · 2023-02-21T12:02:49Z

Please squash your commits.

svenstaro · 2023-02-21T12:08:30Z

Fixes #1039.

Yusuto · 2023-02-21T12:18:17Z

Please squash your commits.

I think I squashed my commits, I hope I did what you mean and didn't mess anything up with the force pushes

Edit: Read some more in the Git Book and I was literally just one chapter away from where it explained how to squash commits properly, so I could have done that instead of the weird git-voodoo that article I found made me do ><

src/renderer.rs

...by having it use the absolute url instead of the breadcrumbs

svenstaro · 2023-02-21T12:53:00Z

It's not necessary for this MR btw, but if you'd like to, you could write a small test to make sure the behavior for this footer is always functional. :)

Yusuto · 2023-02-21T13:02:51Z

It's not necessary for this MR btw, but if you'd like to, you could write a small test to make sure the behavior for this footer is always functional. :)

Oh I'd love to do that Yip!
Would it make sense to just test the function wget_footer by giving it inputs and seeing if it matches a certain output? That's how I'd do it, but I'd consult a testing manual just in case!

Also I couldn't add the tests right now, should I add them in a separate PR, add there here later (squashed or unsquashed?), or for now not at all?

svenstaro · 2023-02-21T13:05:05Z

You could also add the tests in this MR. Don't worry, I'll wait. I just want to release something this week ideally.

I think in this case a unit test of this particular function in the same file makes sense as well as an integration test. we have plenty of examples for both so it shouldn't be that hard. :)

Yusuto · 2023-02-21T13:06:29Z

Yip, sounds gud, Thanks! I'll get to it either later today or tomorrow morning ☀️

The command now has slightly more concise flags, the cut_dir flag gets omitted when it would be zero, if downloading root put files in a folder named the title of the page, use match claues instead of ifs, bit more concise I think

Yusuto · 2023-02-22T10:17:56Z

Oh also I just notice this breaks a bunch of tests, I'll adjust them to check for the new behavior

I honestly just switched the definition and then frantically changed small things based on rusts error messages, but it passes fmt, clippy and tests so I think it's fine. This allow a bit finer control over the URI, but is honetly a bit insignificant.

Yusuto · 2023-02-22T20:06:40Z

I'm done I think!

Fixed footer
Made footer click to copy (I didn't pay attention to accessibility though)
Added some fancy improvements
Added and updated tests

If any issues arise I'll get to them tomorrow, but if nawt then I think this MR is ready to merge!

src/listing.rs

svenstaro · 2023-02-24T00:34:47Z

src/renderer.rs

+    fn test_wget_footer() {
+        let to_html = |x| {
+            format!(
+                r#"<div class="downloadDirectory"><p>Download folder:</p><a class="cmd" title="Click to copy!" style="cursor: pointer;" onclick="navigator.clipboard.writeText(&quot;wget -rcnHp -R 'index.html*' {0}/?raw=true'&quot;)">wget -rcnHp -R 'index.html*' {0}/?raw=true'</a></div>"#,
+                x
+            )
+        };
+        let uri = |x| Uri::try_from(x).unwrap();
+
+        let to_be_tested: String = wget_footer(
+            &uri("https://github.com/svenstaro/miniserve/"),
+            Some("Miniserve"),
+            None,


Why not make all of these test cases into their own unit tests? That way, you can also make it self-documenting by varying the test names.

src/renderer.rs

svenstaro · 2023-02-25T14:53:53Z

Hey, do you plan on fixing up the rest soonish? I'd like to do a release. No worries if you need longer but then it might not make the release.

...discovered when writing the tests. Ran rustfmt, clippy::all, cargo test, everything passed and I hope the tests I wrote are good. Now with 100% less forgotten debug logs!

Yusuto · 2023-02-25T16:48:00Z

Hey, do you plan on fixing up the rest soonish? I'd like to do a release. No worries if you need longer but then it might not make the release.

Oh, I'm not exactly sure what is left to be fixed, it's only the tests, right?

I wrote a comment earlier that I'm very unsure about how to split them apart, whatever approach I try seems to be just kind of not really optimal, or at least not significantly better than just lumping it all in one function and if something fails manually figuring out what it is, so I'm a bit at loss of how to continue there I'm afraid

svenstaro · 2023-03-01T02:54:52Z

Merging as is, thanks! I split the tests the way I imagined it.

Yusuto · 2023-03-07T20:58:16Z

Oké, Thank you! This is my first MR I ever made, I'm very happy to get it accepted ^^

svenstaro · 2023-03-07T21:30:01Z

Thanks for the contribution :) I'd be happy for more contributions of course if you find something you'd like to change.

Yusuto · 2023-03-07T22:43:16Z

No problem!
I will definitely consider it, Yeh! More advanced uploading (like uploading folders and seeing progress) would be pretty heccin practical I think for example! Although I'm also very interested in media processing and am currently looking into that, I'll also look at issues if I get the time ^^

svenstaro reviewed Feb 21, 2023

View reviewed changes

src/renderer.rs Outdated Show resolved Hide resolved

Yusuto force-pushed the wget-fix branch from 5f276d2 to 5881fc3 Compare February 21, 2023 11:41

Yusuto force-pushed the wget-fix branch 2 times, most recently from 4389d7b to a836b46 Compare February 21, 2023 12:17

svenstaro reviewed Feb 21, 2023

View reviewed changes

src/renderer.rs Outdated Show resolved Hide resolved

Yusuto force-pushed the wget-fix branch from a836b46 to 067edc3 Compare February 21, 2023 12:29

Fix the wget footer

56b4cd3

...by having it use the absolute url instead of the breadcrumbs

Yusuto force-pushed the wget-fix branch from 067edc3 to 56b4cd3 Compare February 21, 2023 12:30

Make the wget footer click to copy

85b222f

Yusuto force-pushed the wget-fix branch from 1b22694 to 85b222f Compare February 22, 2023 10:49

Yusuto force-pushed the wget-fix branch from 766a3f0 to 59a704c Compare February 22, 2023 20:01

Yusuto force-pushed the wget-fix branch 2 times, most recently from 44717ab to 4547dc9 Compare February 23, 2023 08:37

svenstaro requested changes Feb 24, 2023

View reviewed changes

svenstaro reviewed Feb 24, 2023

View reviewed changes

src/renderer.rs Outdated Show resolved Hide resolved

svenstaro reviewed Feb 24, 2023

View reviewed changes

src/renderer.rs Show resolved Hide resolved

Yusuto force-pushed the wget-fix branch from 4547dc9 to ad430d2 Compare February 24, 2023 09:47

Added tests and small fixes

511d4db

...discovered when writing the tests. Ran rustfmt, clippy::all, cargo test, everything passed and I hope the tests I wrote are good. Now with 100% less forgotten debug logs!

Yusuto force-pushed the wget-fix branch from ad430d2 to 511d4db Compare February 25, 2023 16:46

svenstaro merged commit a93170d into svenstaro:master Mar 1, 2023

svenstaro added a commit that referenced this pull request Mar 1, 2023

Add CHANGELOG entry for #1043

471bb77

jgranduel mentioned this pull request Apr 18, 2023

curl footer? #1103

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the wget footer #1043

Fix the wget footer #1043

Yusuto commented Feb 20, 2023

svenstaro commented Feb 21, 2023

svenstaro commented Feb 21, 2023

Yusuto commented Feb 21, 2023 •

edited

Loading

svenstaro commented Feb 21, 2023

Yusuto commented Feb 21, 2023

svenstaro commented Feb 21, 2023

Yusuto commented Feb 21, 2023

Yusuto commented Feb 22, 2023

Yusuto commented Feb 22, 2023 •

edited

Loading

svenstaro Feb 24, 2023

svenstaro commented Feb 25, 2023

Yusuto commented Feb 25, 2023

svenstaro commented Mar 1, 2023

Yusuto commented Mar 7, 2023

svenstaro commented Mar 7, 2023 •

edited

Loading

Yusuto commented Mar 7, 2023

Fix the wget footer #1043

Fix the wget footer #1043

Conversation

Yusuto commented Feb 20, 2023

svenstaro commented Feb 21, 2023

svenstaro commented Feb 21, 2023

Yusuto commented Feb 21, 2023 • edited Loading

svenstaro commented Feb 21, 2023

Yusuto commented Feb 21, 2023

svenstaro commented Feb 21, 2023

Yusuto commented Feb 21, 2023

Yusuto commented Feb 22, 2023

Yusuto commented Feb 22, 2023 • edited Loading

svenstaro Feb 24, 2023

Choose a reason for hiding this comment

svenstaro commented Feb 25, 2023

Yusuto commented Feb 25, 2023

svenstaro commented Mar 1, 2023

Yusuto commented Mar 7, 2023

svenstaro commented Mar 7, 2023 • edited Loading

Yusuto commented Mar 7, 2023

Yusuto commented Feb 21, 2023 •

edited

Loading

Yusuto commented Feb 22, 2023 •

edited

Loading

svenstaro commented Mar 7, 2023 •

edited

Loading