feat: yank magnet without metadata and trackers

[slundi]

Close: #37

[Beastwick18]

LGTM 👍, just would make sure to handle errors with Url::parse instead of unwrapping

[slundi]

Hello,

Thanks for your complete review, I just had to copy-paste and test :)

[brian6932]

This can be done in a different PR, but want to mention anyway, this bool should be applied to client handlers as well as yanks. Either the bool should be renamed more generically, and used for both, or 2 bools can be made.

nyaa/src/client/transmission.rs

Line 117 in c496c50

None | Some(true) => item.magnet_link.to_owned(),

nyaa/src/client/rqbit.rs

Line 80 in c496c50

true => item.magnet_link.to_owned(),

nyaa/src/client/default_app.rs

Line 26 in c496c50

true => item.magnet_link.to_owned(),

nyaa/src/client/cmd.rs

Line 40 in c496c50

.sub("{magnet}", &item.magnet_link)

[slundi]

I can improve it, it shouldn't be difficult

[slundi]

@brian6932

I was beginning to implement per client but:

impl DownloadClient for RqbitClient {
    async fn download(
        item: Item,
        conf: ClientConfig,
        client: reqwest::Client,
    ) -> SingleDownloadResult {
        let conf = match conf.rqbit.clone() {
            Some(q) => q,
            None => {
                return SingleDownloadResult::error("Failed to get rqbit config");
            }
        };
        let link = match conf.use_magnet.unwrap_or(true) {
            true => {
                if let Some(full) = conf.yank_full_magnet {
                    if full {
                        item.magnet_link.to_owned()
                    } else {
                        match minimal_magnet_link(item.magnet_link.clone()){
                            Ok(magnet) => magnet.to_owned(),
                            Err(_) => todo!(),  // context not available here
                        }
                    }
                } else {
                    item.magnet_link.to_owned()
                }
            }
            false => item.torrent_link.to_owned(),
        };
        // ...
    }
}

But, in case of error, I don't have the context (to do something like return ctx.notify_error(e)), how should it be solved? return the item.magnet_link anyway?

[brian6932]

how should it be solved? return the item.magnet_link anyway?

I'm no rust expert, but it sounds pretty ok to me. There's not much of a chance for error. At that point it's an issue with nyaa sending non-compliant magnet links (I don't think it's possible for any listings to not have one in the first place). The most I can say about saftey's that it may be good to add a BTMH magnet v2 URI test case, for forwards compatibility.

[Beastwick18]

Everything LGTM functionality wise, so I can implement the changes mentioned after merging if that sounds good!

[Beastwick18]

Everything looks good, thanks for adding this!

[Beastwick18]

Whoops, just noticed the condition is backwards for keycombo 'ym'. It yanks the minimal link when yank_full_magnet = true and the full one when yank_full_magnet = false. Fixed that 8026e52