Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repository in the config is not used anymore #438

Open
s3i7h opened this issue Apr 12, 2024 · 1 comment
Open

repository in the config is not used anymore #438

s3i7h opened this issue Apr 12, 2024 · 1 comment

Comments

@s3i7h
Copy link

s3i7h commented Apr 12, 2024

following up #420

I ended up there while bisecting repositoryrepositoryBase 's silent (and partialy a breaking) change. I believe this PR removed support of setting repository in the configuration file, which makes tldr fallback to default assets instead of the specified one. To handle this:

  • Update documentation according to the code (to supply a base url and construct the actual download inside tldr) and note it's a breaking change
  • Add a condition to allow all-language download if repository was specified (this makes it backwards compatible)

I think one of the two (prefferably both) of the above measures should be taken. (I think the second one should be implemented with a deprecation warning of some form as it's inefficient for both the client and the server)(if we're going to implement it)

Affected codes between prior to #420 and the current main branch (7ad3c5a..c58c92c):

  • config.json:

    "repository": "https://tldr.sh/assets/tldr.zip",

    "repositoryBase": "https://tldr.sh/assets/tldr-pages",

  • lib/cache.js

    .then(() => {
    // Download and extract cache data to temporary folder
    return remote.download(tempFolder);
    })

    .then(() => {
    // Download and extract cache data to temporary folder
    return Promise.allSettled(this.config.languages.map((lang) => {
    return remote.download(tempFolder, lang);
    }));
    })

  • lib/remote.js

    exports.download = (path) => {
    const url = config.get().repository;

    exports.download = (loc, lang) => {
    // 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;

I don't have time currently to make a PR on my own now, so I would appreciate if someone could handle this. However if this remains for a month or two, maybe I can put my hands on it.

@agnivade
Copy link
Member

@kbdharun @owenvoke - Wondering if you guys can take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants