trakt / script.trakt

Trakt.tv movie and TV show scrobbler for Kodi
GNU General Public License v2.0
318 stars 149 forks source link

Allow to override global proxy settings #581

Closed oliv3r closed 2 years ago

oliv3r commented 2 years ago

There are various use-cases where we want to configure proxy settings within trakt. E.g. Kodi/the host may be setup to connect to a Socks5 proxy, which script.trakt currently does not support. Additionally the user may want to use a different proxy (or just for trakt) then the Kodi/host configuration has set up. As this is feature for expert users, this should be hidden behind the expert settings level.

To accomplish the above, a second commit exists in this merge request, to migrate the settings.xml file to the newer kodi format, which allows to hide settings at different levels.

oliv3r commented 2 years ago

@razzeee what are the next steps to get this merged now? Anything else I need to do?

razzeee commented 2 years ago

I need to find time to do a final settings test (or run time tests) I only had a look at the code above.

razzeee commented 2 years ago

The scrobbling and general categories seem to have problems.

  1. scrobbling, changed order and a seemingly arbitrary grouping (draws a line in the middle)
  2. general, seems like it shows the hidden settings on expert level? there are a bunch of settings, that are just empty with no title.
oliv3r commented 2 years ago

@razzeee It seems to be working here, but I've not extensively tested each and every setting.

1) As for scrobbeling; https://github.com/trakt/script.trakt/pull/581/commits/8d975cc5f91b927bcb3def7558df1e93fef022bf#diff-8e5952d4ad33bfc8e7b5b8739c8f898dd3f950d0f2548bc09f901489f50f6339R417 defines the group and indeed https://github.com/trakt/script.trakt/pull/581/commits/8d975cc5f91b927bcb3def7558df1e93fef022bf#diff-8e5952d4ad33bfc8e7b5b8739c8f898dd3f950d0f2548bc09f901489f50f6339R434 is the second group defined. Could be copy paste error (or unconscious :) It might be as it seemed logical to group the scrobbeling types together? I don't remember, I'll remove the group and double check the order again. Come to think of it, this was one of the sections I was playing with and maybe I intended to offer this (later) as an settings improvement. Anyway, it's fixed :)

The blank line is because there's no name (label) for this section/group (yet).

2) This is 'just how kodi's setting system works' I guess /I don't think you can define the level on a category/group level, just on an individual setting level. My kodi won't show sections (latest libreelec) that have no settings. So it looks/feels quite natural. But the default levels are up to you I suppose what you want. I think the current 'makes sense' where the basic only has connectivity settings, and 'standard' (i think the kodi default) will show up most 'flags'.

razzeee commented 2 years ago

Scrobbling seems fine

image

General still needs work

oliv3r commented 2 years ago

Scrobbling seems fine

General still needs work

Hey @razzeee, I only did the language file for en_US; not sure what was needed. I figured once merged transifex would add them. What is the process normally here? Should I add all translations entries? (I head tail -n 10 en_US >> $file would work) ...

razzeee commented 2 years ago

I don't think that's the problem. I would think those are the hidden config fields, that are now configured to show up (by mistake I assume)

oliv3r commented 2 years ago

I don't think that's the problem. I would think those are the hidden config fields, that are now configured to show up (by mistake I assume)

@razzeee but they look perfectly like the proxy stuff, e.g. URL (blank string) port (0) etc. I put the translations in my en_GB file as well for testing, and i see the actual text, which is as expected either way, as it is supposed to show up in 'expert' mode?

I get the same as your screenshot; but when I do tail -n 40 resource.language.en_US/strings.po >> resource.language.en_GB/strings.po it looks fine?

razzeee commented 2 years ago

Your right, I didn't realize you did US instead of UK. As far as I know we need to change that, to have translations working.

oliv3r commented 2 years ago

Your right, I didn't realize you did US instead of UK. As far as I know we need to change that, to have translations working.

Ok, i'll do a quick shell script and do all langs; done @razzeee on the upside, it made the review easier :)

for future googlers, what I did was

for _dir in *; do tail -n 40 resource.language.en_US/strings.po >> ${_dir}/strings.po; done
git restore resource.language.en_US/strings.po
git add */strings.po
razzeee commented 2 years ago

@gade01 can we add the translations, as proposed or would that mess up the translation flow, as it expects to do it on it's own?

oliv3r commented 2 years ago

@gade01 can we add the translations, as proposed or would that mess up the translation flow, as it expects to do it on it's own?

I didn't add the translations themselves, just the untranslated entries, which should mean, if untranslated, they show up as english, once a translating service (transifex at all) drops by, they list them as untranslated strings until someone fills it in I suppose.

gade01 commented 2 years ago

@razzeee @oliv3r Thanks for reaching out :) New translations should be added only to en_gb file. Weblate will add them the correct way to all other language files.

Right now they are added after the commented out lines for some languages,

Also, please add a line break before new strings in en_gb file: https://github.com/trakt/script.trakt/pull/581/files#diff-12c1b153e8d2d6703d5a8cb60ea98f77680652daf82ae49903767a212d12bfe3R740

gade01 commented 2 years ago

Please merge any open Weblate PR before merging this one, thanks.

https://github.com/trakt/script.trakt/pull/583

Otherwise it will cause merge coonflicts at Weblate.

oliv3r commented 2 years ago

@gade01 Done, I removed the other language and only left the en_GB one (after the newline fix; turns out, there was no line-ending at all :( There is now again.

@razzeee as #583 is now merged, I have also rebased as well, so should be ready to go :)

razzeee commented 2 years ago

Thanks for sticking with us :)

oliv3r commented 2 years ago

No problem, I hope you value the contribution :)