Fetch individual translation archives for cache (#217)

[SaurabhDRao]

Fetch individual translation archives for cache based on the environment variable configuration (#217)

[kbdharun]

LGTM, Thanks for your contribution.

[acuteenvy]

This works, but it downloads only the language set in LANG or TLDR_LANGUAGE - LANGUAGE is ignored. Also, it doesn't always download English, and that causes the client to download a single page from GitHub when a translation is not available. Is this expected?

[kbdharun]

Hi @SaurabhDRao, Any updates on this?

[SaurabhDRao]

I am extremely sorry, as I don't remember seeing these comments earlier.

Anyways, from what I checked, LANGUAGE variable is supposed to hold a list of user preferred languages and I believe what you are asking is to download the data for all the languages that are set in LANGUAGE variable ?

And also if a translation is unavailable for a language that is set in any of the language variables, we need to go ahead and download the data for English anyways ?

[kbdharun]

I am extremely sorry, as I don't remember seeing these comments earlier.

No issues, thanks for your work.

Anyways, from what I checked, LANGUAGE variable is supposed to hold a list of user preferred languages and I believe what you are asking is to download the data for all the languages that are set in LANGUAGE variable ?

And also if a translation is unavailable for a language that is set in any of the language variables, we need to go ahead and download the data for English anyways ?

Yes, fetching only the languages specified in the environment variable, if a translated page isn't available display (fallback to) the English page with a warning that a translated page doesn't exist for the command in their language.

Additionally, the language flag when called with a language not set in the environment variable, we could add a check that says language should be set there or if it's done already, then refresh the cache using the update command.

[kbdharun]

Kinda curious, shouldn't lines 28-31 be updated for the same, instead of fetching the entire archive?

Reference:

DOWNLOAD_CACHE_LOCATION = os.environ.get(
    'TLDR_DOWNLOAD_CACHE_LOCATION',
    'https://tldr-pages.github.io/assets/tldr.zip'
)

[SaurabhDRao]

Actually, I don't think that is required. If you have a look at line number 382

cache_location = f"{DOWNLOAD_CACHE_LOCATION[:-4]}-pages.{language}.zip"

you can see that the cache location will dynamically get updated as https://tldr-pages.github.io/assets/tldr-pages.{language}.zip

And since there is a call to get_language_list() which will return atleast "en" as a language, we would still be getting only language specific cache.

But one thing to note is that if the cache is being pulled from a different location, set in TLDR_DOWNLOAD_CACHE_LOCATION, it should follow similar path structure as in -pages.{language}.zip.
Do you think its fine or do you want me to make line 382 a bit more generic ?

[kbdharun]

Seems like linter is set to accept only 88 characters per line, feel free to update line 400, 401 for the same.

[vitorhcl]

LGTM, thanks for your contribution!

[acuteenvy]

Works as expected.