mateoradman / bazarr-bulk

Bazarr Bulk Actions CLI - Bulk sync your subtitles with ease
https://crates.io/crates/bazarr-bulk
MIT License
30 stars 1 forks source link

fix(#7): add "base" key in config to specify base url #23

Closed matt1432 closed 1 month ago

matt1432 commented 1 month ago

I tested this and it worked with the example I added in ./examples.

If you feel like "base" is too vague feel free to change the name. It could be base_url but you already use that name elsewhere so I wasn't sure what to do with that.

I've never coded in rust before so I'm not sure if I needed to add this line:

.set_default("base", "")?

or is that already default?

Finally, it might be important to say somewhere that the leading slash in the base url is needed, or we could figure out a way to make it not needed. Not sure what the proper way to do this would be.

mateoradman commented 1 month ago

Hi @matt1432, many thanks for this pull request. I will quickly take a look at code, and test it. After that, I will address your comments regarding the naming convention, and making the leading slash optional.

Adding a line .set_default("base", "")? is a good idea, to set the base url to an empty string if not present in the config.json file.

mateoradman commented 1 month ago

Thanks @matt1432, I've addressed both points in my last commit. The baseUrl field in the config.json does not require a leading slash anymore. I've also renamed the base_url to bazarr_url and base to base_url to follow a nice naming convention in accordance with Bazarr.