Only download cache for system languages and English #420

vivekjoshi556 · 2023-10-04T18:28:58Z

Description

Fixes #419
The task at hand was to update the client so that it would only download the zip files for languages that were supported by the user system. In any case, files in english should be downloaded to act as backup.

Explanation of Solution

While doing this I realized that the unzip library being used was causing issue as it was emitting the close event on extractor twice and because of it the extraction wasn't completing and some of the files were not getting extracted. Trying to resolve it I saw similiar issues on the official repo and the issue wasn't resolved. So I decided to change the npm package to unzip.
It uses both ll & cc to download the correct zip file whenever possible.
Is there a way to determine which languages have different version available (like pt has pt_BR, pt_PT), right now I have hard-coded the values in util.js and it will need to be updated in case other versions for other languages are added.

Checklist

Please review this checklist before submitting a pull request.

Code compiles correctly
Created tests, if possible
All tests passing (npm run test:all)
Extended the README / documentation, if necessary

…kup | Changed the package for Unzipping

agnivade · 2023-10-05T04:24:49Z

Can we add some tests please?

vivekjoshi556 · 2023-10-05T04:58:14Z

Can we add some tests please?

@agnivade Sure I'll add the tests
Can you please help me with the failing job in the workflow? All the jobs passed when I made the first commit before the PR, but now one of them is failing. It happened in the repo I forked and when I tried to run the job again alone it passed.

All passed again. 🤔

kbdharun · 2023-10-05T06:15:09Z

Btw, @vivekjoshi556

Workflow requiring approval before running is enabled for first time contributors so that maintainers can review the code before running the action. (This is to minimize multiple failed runs). After this PR is merged, in future the workflow will run by default for you (without requiring approval).

All passed again. 🤔

The reason for it to pass was that I approved your action run an hour ago when you made the comment.

vivekjoshi556 · 2023-10-05T06:26:45Z

Workflow requiring approval before running is enabled for first time contributors so that maintainers can review the code before running the action. (This is to minimize multiple failed runs). After this PR is merged, in future the workflow will run by default for you (without requiring approval).

I understand they require approval. I was talking about the last time the jobs ran for my changes.
Actions run link.
In this run the job for ubuntu-latest, 20.x failed. All others passed.

kbdharun · 2023-10-05T06:33:19Z

I understand they require approval. I was talking about the last time the jobs ran for my changes.
Actions run link.
In this run the job for ubuntu-latest, 20.x failed. All others passed.

Oh, it is possible that the action run was cancelled as it reached 6 hours limit (default for action runs in GitHub's runners).

…Settings

vivekjoshi556 · 2023-10-09T17:31:02Z

@kbdharun & @agnivade please check this PR & let me know if anything else needs to be updated.

lib/cache.js

lib/config.js

lib/remote.js

test/remote.spec.js

config.json

package.json

vivekjoshi556 · 2023-10-18T18:53:18Z

@MasterOdin I am done with the mentioned changes. Please review the PR.

kbdharun · 2023-10-24T16:10:58Z

lib/remote.js

+  // If the lang is english then keep the url simple, otherwise add language.
+  const suffix = (lang === 'en' ? '' : '.' + lang);
+  const url = config.get().repositoryBase + suffix + '.zip';
+  const folderName = path.join(loc, 'pages' + suffix);
+  const REQUEST_TIMEOUT = 10000;


Regarding this, we recently added support for the English archive in the same format tldr-pages-en.zip (after a request from tealdeer's maintainer). Can it be implemented here instead? (This will officially land in the upcoming client spec 2.1)

sure should be easy.

It should be noted that only the archive name should be generalized here, while still leaving in place that the english archive is extracted to the pages directory. Changing that behavior is a much larger one, which should be done in a separate PR that's focused purely on that, as will also have to take into account backwards compatibility.

So, won't it be better to handle this entirely in the same (other) PR?

I'd say so, but not sure how much @kbdharun wants to see the archive name changed here. The old zip will continue to work anyway for the time being that it's not exactly urgent to make that change either.

kbdharun · 2023-10-24T16:59:00Z

I guess this PR can be merged first, then I will rebase mine with this.

Fix: Download Only the files for Required Language And English as Bac…

9d14c14

…kup | Changed the package for Unzipping

kbdharun requested review from agnivade, MasterOdin, owenvoke, SethFalco and kbdharun October 4, 2023 18:32

kbdharun mentioned this pull request Oct 5, 2023

feat (3.4.0): add platform option, Android and BSD platforms #421

Merged

4 tasks

Test: Added Tests to Check for Downloads based on different Language …

7a09771

…Settings

vivekjoshi556 marked this pull request as draft October 7, 2023 12:17

Fix: Fixed Test Cases | Feat: Included use of LANGUAGE env variable

aaa9947

vivekjoshi556 marked this pull request as ready for review October 8, 2023 17:48

MasterOdin reviewed Oct 13, 2023

View reviewed changes

Fix: Review Changes

fadd927

vivekjoshi556 requested a review from MasterOdin October 18, 2023 18:53

kbdharun added the hacktoberfest-accepted PRs which are eligible for hacktoberfest but won't get merged before 31st label Oct 24, 2023

MasterOdin added 2 commits October 24, 2023 11:54

Promise.all change

04a8cf2

Split utils tests to its own file

6a4f964

MasterOdin changed the title ~~Fix: Download Only the files for Required Language And English as Backup | Changed the package for Unzipping~~ Only download cache for system languages and english Oct 24, 2023

MasterOdin approved these changes Oct 24, 2023

View reviewed changes

kbdharun reviewed Oct 24, 2023

View reviewed changes

kbdharun changed the title ~~Only download cache for system languages and english~~ Only download cache for system languages and English Oct 24, 2023

kbdharun approved these changes Oct 24, 2023

View reviewed changes

MasterOdin merged commit d3f0309 into tldr-pages:main Oct 24, 2023
18 checks passed

vivekjoshi556 deleted the issue_419 branch October 24, 2023 17:55

code2933 mentioned this pull request Oct 26, 2023

fix: Update command does not work with proxy #423

Open

4 tasks

dbrgn mentioned this pull request Feb 11, 2024

Only download pages matching the user's languages dbrgn/tealdeer#345

Draft

s3i7h mentioned this pull request Apr 12, 2024

repository in the config is not used anymore #438

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only download cache for system languages and English #420

Only download cache for system languages and English #420

vivekjoshi556 commented Oct 4, 2023 •

edited

Loading

agnivade commented Oct 5, 2023

vivekjoshi556 commented Oct 5, 2023 •

edited

Loading

kbdharun commented Oct 5, 2023 •

edited

Loading

vivekjoshi556 commented Oct 5, 2023

kbdharun commented Oct 5, 2023

vivekjoshi556 commented Oct 9, 2023 •

edited

Loading

vivekjoshi556 commented Oct 18, 2023

kbdharun Oct 24, 2023 •

edited

Loading

vivekjoshi556 Oct 24, 2023

MasterOdin Oct 24, 2023

vivekjoshi556 Oct 24, 2023

MasterOdin Oct 24, 2023

kbdharun commented Oct 24, 2023 •

edited

Loading

Only download cache for system languages and English #420

Only download cache for system languages and English #420

Conversation

vivekjoshi556 commented Oct 4, 2023 • edited Loading

Description

Explanation of Solution

Checklist

agnivade commented Oct 5, 2023

vivekjoshi556 commented Oct 5, 2023 • edited Loading

kbdharun commented Oct 5, 2023 • edited Loading

vivekjoshi556 commented Oct 5, 2023

kbdharun commented Oct 5, 2023

vivekjoshi556 commented Oct 9, 2023 • edited Loading

vivekjoshi556 commented Oct 18, 2023

kbdharun Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

vivekjoshi556 Oct 24, 2023

Choose a reason for hiding this comment

MasterOdin Oct 24, 2023

Choose a reason for hiding this comment

vivekjoshi556 Oct 24, 2023

Choose a reason for hiding this comment

MasterOdin Oct 24, 2023

Choose a reason for hiding this comment

kbdharun commented Oct 24, 2023 • edited Loading

vivekjoshi556 commented Oct 4, 2023 •

edited

Loading

vivekjoshi556 commented Oct 5, 2023 •

edited

Loading

kbdharun commented Oct 5, 2023 •

edited

Loading

vivekjoshi556 commented Oct 9, 2023 •

edited

Loading

kbdharun Oct 24, 2023 •

edited

Loading

kbdharun commented Oct 24, 2023 •

edited

Loading