-
Notifications
You must be signed in to change notification settings - Fork 5
provide shortened url if a shortener is used on the host and --shorte… #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…n is specified This takes advantage of if the host has shlink or yourl configured via it's local proxy (shortenviayourls or shortenviashlink) and provides the shortened url if found. If --shorten is applied and there is no proxy it will fall back to the default url. Likewise if a config file has --shorten and you want to retrieve the full url --no-shorten can be used.
Mydayyy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution, highly appreciated! I noted down a few points, happy to hear your thoughts on it
| directories = "5.0.1" | ||
| log = "0.4.22" | ||
| scraper = "0.21.0" | ||
| ureq = "2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is unused, isn't it? If yes we can remove it
| match try_method(opts, long_url, "shortenviayourls") { | ||
| Ok(u) => Ok(u), | ||
| Err(e1) => { | ||
| log::debug!("YOURLS shorten failed ({e1:?}), trying shlink…"); | ||
|
|
||
| // 2) fall back to shlink proxy | ||
| match try_method(opts, long_url, "shortenviashlink") { | ||
| Ok(u) => Ok(u), | ||
| Err(e2) => { | ||
| // preserve useful debug, but return a single error | ||
| log::debug!("Shlink shorten also failed ({e2:?})"); | ||
| Err(PasteError::InvalidData) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a nested retry, I would propose a different construct:
turn the methods into an array and loop over it, aborting on the first non-None result, this can be done with
https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.find_map
| if let Some(href_pos) = after_id.find("href=\"") { | ||
| let after_href = &after_id[href_pos + "href=\"".len()..]; | ||
| if let Some(end) = after_href.find('"') { | ||
| let url = &after_href[..end]; | ||
| if url.starts_with("https://") || url.starts_with("http://") { | ||
| return Ok(url.to_string()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let Some(href_pos) = after_id.find("href='") { | ||
| let after_href = &after_id[href_pos + "href='".len()..]; | ||
| if let Some(end) = after_href.find('\'') { | ||
| let url = &after_href[..end]; | ||
| if url.starts_with("https://") || url.starts_with("http://") { | ||
| return Ok(url.to_string()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as below should be possible here to reduce code duplication, since I believe the only difference is ' or ":
Turn those into an array and use find_map (like below) to get the first non-None result
…n is specified
This takes advantage of if the host has shlink or yourl configured via it's local proxy (shortenviayourls or shortenviashlink) and provides the shortened url if found. If --shorten is applied and there is no proxy it will fall back to the default url. Likewise if a config file has --shorten and you want to retrieve the full url --no-shorten can be used.
Tested on host with both shlink and yourls configured.