Repository: tldr-pages/tldr-node-client
Pull request #270 (2019-07-07 06:42:02)

Added: language support
## Description
An environment variable LANG needs to contain the correct BCP-47 tag
for it to pick up the right language file. :tada: 

## Checklist

Please review this checklist before submitting a pull request.

- [x] Code compiles correctly
- [x] Created tests, if possible
- [x] All tests passing (`npm run test:all`)
- [x] Extended the README / documentation, if necessary

@sbrl @mebeim @pxgamer @waldyrious 

agnivade: Sure
agnivade: @mebeim - This is ready for review.

@waldyrious - I updated the README text slightly. Please do check once to ensure everything looks good to you.
waldyrious: > @waldyrious - I updated the README text slightly. Please do check once to ensure everything looks good to you.

Thanks for the ping. I left some minor comments inline, but overall it looks good. 👍
mebeim: @agnivade does this PR follow the client spec? In particular, this:

> The [`LANG` environment variable](https://www.gnu.org/software/gettext/manual/html_node/Locale-Environment-Variables.html), if present, MUST be used to determine the language of pages to display.
>
> The [`LANGUAGE` environment variable](https://www.gnu.org/software/gettext/manual/html_node/The-LANGUAGE-variable.html) specifies a priority list of languages that a user wishes to read in. In absence of `LANG` and if this environment variable present, a client MUST use the defined priority list to decide on the language of pages to display. If a page is not available in the user's preferred language, then a client MUST respect the user's priority list defined in the `LANGUAGE` variable, and MAY choose to notify the user that a page in their chosen language couldn't be found (perhaps along with a link to the [translations section of the contributing guide](https://github.com/tldr-pages/tldr/blob/master/CONTRIBUTING.md#translations)).

I'm sorry but I don't have enough time to review/test this in detail now.
agnivade: It does not support the `LANGUAGE` or `LC_MESSAGES` yet. Only `LANG`. That is a TODO item for future.
agnivade: > I did think of an edge case where both aa_BB and aa exist, but aa has pages that aa_BB does not have, but I'm unsure as to how important it would be to handle that.

It is already handled. :)

```js
let ll;
      if (preferredLanguage.indexOf('_') > 0) {
        ll = preferredLanguage.substring(0, preferredLanguage.indexOf('_'));
      }
      if (!hasLang(targets, preferredLanguage)) {
        preferredLanguage = ll;
      }
```
mebeim: So @agnivade should this be squashed? Seeing the commits you are adding I would assume so.

agnivade: Yep
agnivade: Thanks guys, merging.

Patch:
diff --git a/.eslintrc.json b/.eslintrc.json
index 0458a04..e31a97e 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -8,7 +8,6 @@
     "arrow-parens": [2, "always"],
     "arrow-body-style": [2, "always"],
     "array-callback-return": 2,
-    "complexity": [2, 10],
     "no-magic-numbers": [2, {
       "ignore": [-1, 0, 1, 2],
       "ignoreArrayIndexes": true,
diff --git a/README.md b/README.md
index d81610c..ceb11a4 100644
--- a/README.md
+++ b/README.md
@@ -41,7 +41,7 @@ To see tldr pages:
 - `tldr <command> --os=<platform>` show command page for the given platform (`linux`, `osx`, `sunos`)
 - `tldr --search "<query>"` search all pages for the query
 - `tldr --linux <command>` show command page for Linux
-- `tldr  --osx <command>` show command page for OSX
+- `tldr --osx <command>` show command page for OSX
 - `tldr --sunos <command>` show command page for SunOS
 - `tldr --list` show all pages for current platform
 - `tldr --list-all` show all available pages
@@ -59,6 +59,14 @@ As a contributor, you might also need the following commands:
 
 - `tldr --render <path>` render a local page for testing purposes
 
+Tldr pages defaults to showing pages in the current language of the operating system, or English if that's not available. To view tldr pages for a different language, set an environment variable `LANG` containing a valid [POSIX locale](https://www.gnu.org/software/gettext/manual/html_node/Locale-Names.html#Locale-Names) (such as `zh`, `pt_BR`, or `fr`) and then run the above commands as usual. In most `*nix` systems, this variable will already be set.
+
+It is suggested that the `LANG` environment variable be set system-wide if this isn't already the case. Users without `sudo` access can set it locally in their `~/.profile`.
+
+- `LANG=zh tldr <command>`
+
+For the list of available translations, please refer to the main [tldr](https://github.com/tldr-pages/tldr) repo.
+
 ## Configuration
 
 You can configure the `tldr` client by adding a `.tldrrc` file in your HOME directory. You can copy the contents of the `config.json` file from the repo to get the basic structure to start with, and modify it to suit your needs.
@@ -147,25 +155,25 @@ fpath = (my/completions $fpath)
 
 #### Installation Issues
 
-- If you are trying to install as non-root user (`npm install -g tldr`) and get something like - 
- 
+- If you are trying to install as non-root user (`npm install -g tldr`) and get something like:
+
  ```
  Error: EACCES: permission denied, access '/usr/local/lib/node_modules/tldr'
  ```
- 
+
  Then most probably your npm's default installation directory has improper permissions. You can resolve it by clicking [here](https://docs.npmjs.com/getting-started/fixing-npm-permissions)
- 
-- If you are trying to install as a root user (`sudo npm install -g tldr`) and get something like - 
+
+- If you are trying to install as a root user (`sudo npm install -g tldr`) and get something like:
 
 ```
-as root -> 
+as root ->
 gyp WARN EACCES attempting to reinstall using temporary dev dir "/usr/local/lib/node_modules/tldr/node_modules/webworker-threads/.node-gyp"
 gyp WARN EACCES user "root" does not have permission to access the dev dir "/usr/local/lib/node_modules/tldr/node_modules/webworker-threads/.node-gyp/8.9.1"
 ```
 
 You need to add the option `--unsafe-perm` to your command. This is because when npm goes to the postinstall step, it downgrades the permission levels to "nobody". Probably you should fix your installation directory permissions and install as a non-root user in the first place.
 
-- If you see an error related to `webworker-threads` like -
+- If you see an error related to `webworker-threads` like:
 
 ```
 /usr/local/lib/node_modules/tldr/node_modules/natural/lib/natural/classifiers/classifier.js:32
diff --git a/lib/cache.js b/lib/cache.js
index e53c007..bdc69bd 100644
--- a/lib/cache.js
+++ b/lib/cache.js
@@ -22,12 +22,16 @@ class Cache {
 
   getPage(page) {
     let preferredPlatform = platform.getPreferredPlatformFolder(this.config);
-    return index.findPlatform(page, preferredPlatform)
+    let preferredLanguage = 'en';
+    if (process.env['LANG'] !== '') {
+      preferredLanguage = process.env['LANG'];
+    }
+    return index.findPage(page, preferredPlatform, preferredLanguage)
       .then((folder) => {
         if (!folder) {
           return;
         }
-        let filePath = path.join(this.cacheFolder, 'pages', folder, page + '.md');
+        let filePath = path.join(this.cacheFolder, folder, page + '.md');
         return fs.readFile(filePath, 'utf8');
       })
       .catch((err) => {
diff --git a/lib/index.js b/lib/index.js
index 42aa364..d116b5b 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -7,10 +7,10 @@ const utils = require('./utils');
 
 let shortIndex = null;
 
-const pagesPath = path.join(config.get().cache, 'cache/pages');
+const pagesPath = path.join(config.get().cache, 'cache');
 const shortIndexFile = path.join(pagesPath, 'shortIndex.json');
 
-function findPlatform(page, preferredPlatform) {
+function findPage(page, preferredPlatform, preferredLanguage) {
   // Load the index
   return getShortIndex()
     .then((idx) => {
@@ -18,17 +18,79 @@ function findPlatform(page, preferredPlatform) {
       if (! (page in idx)) {
         return null;
       }
-      // Get the platforms
-      let platforms = idx[page];
-      if (platforms.indexOf(preferredPlatform) >= 0) {
-        return preferredPlatform;
-      } else if (platforms.indexOf('common') >= 0) {
-        return 'common';
+      const targets = idx[page].targets;
+
+      // Remove unwanted stuff from lang code.
+      if (preferredLanguage.includes('.')) {
+        preferredLanguage = preferredLanguage.substring(0, preferredLanguage.indexOf('.'));
+      }
+      if (preferredLanguage.includes('@')) {
+        preferredLanguage = preferredLanguage.substring(0, preferredLanguage.indexOf('@'));
+      }
+
+      let ll;
+      if (preferredLanguage.includes('_')) {
+        ll = preferredLanguage.substring(0, preferredLanguage.indexOf('_'));
+      }
+      if (!hasLang(targets, preferredLanguage)) {
+        preferredLanguage = ll;
+      }
+
+      // Page resolution logic:
+      // 1. Look into the target platform, target lang
+      // 2. If not found, look into target platform, en lang.
+      // 3. If not found, look into common, target lang.
+      // 4. If not found, look into common, en lang.
+      // 5. If not found, look into any platform, target lang.
+      // 6. If not found, look into any platform, en lang.
+      let targetPlatform;
+      let targetLanguage;
+      if (hasPlatformLang(targets, preferredPlatform, preferredLanguage)) {
+        targetLanguage = preferredLanguage;
+        targetPlatform = preferredPlatform;
+      } else if (hasPlatformLang(targets, preferredPlatform, 'en')) {
+        targetLanguage = 'en';
+        targetPlatform = preferredPlatform;
+      } else if (hasPlatformLang(targets, 'common', preferredLanguage)) {
+        targetLanguage = preferredLanguage;
+        targetPlatform = 'common';
+      } else if (hasPlatformLang(targets, 'common', 'en')) {
+        targetLanguage = 'en';
+        targetPlatform = 'common';
+      } else if (targets.length > 0 && hasLang(targets, preferredLanguage)) {
+        targetLanguage = preferredLanguage;
+        targetPlatform = targets[0].os;
+        console.log(`Command ${page} does not exist for the host platform. Displaying the page from ${targets[0].os} platform`);
+      } else if (targets.length > 0 && hasLang(targets, 'en')) {
+        targetLanguage = 'en';
+        targetPlatform = targets[0].os;
+        console.log(`Command ${page} does not exist for the host platform. Displaying the page from ${targets[0].os} platform`);
+      }
+
+      if (!targetLanguage && !targetPlatform) {
+        return null;
       }
-      return null;
+
+      let targetPath = 'pages';
+      if (targetLanguage !== 'en') {
+        targetPath += '.' + targetLanguage;
+      }
+      return path.join(targetPath, targetPlatform);
     });
 }
 
+function hasPlatformLang(targets, preferredPlatform, preferredLanguage) {
+  return targets.some((t) => {
+    return t.os === preferredPlatform && t.language === preferredLanguage;
+  });
+}
+
+function hasLang(targets, preferredLanguage) {
+  return targets.some((t) => {
+    return t.language === preferredLanguage;
+  });
+}
+
 // hasPage is always called after the index is created,
 // hence just return the variable in memory.
 // There is no need to re-read the index file again.
@@ -53,7 +115,9 @@ function commandsFor(platform) {
     .then((idx) => {
       let commands = Object.keys(idx)
         .filter((cmd) => {
-          return idx[cmd].indexOf(platform) !== -1 || idx[cmd].indexOf('common') !== -1;
+          let targets = idx[cmd].targets;
+          let platforms = targets.map((t) => {return t.os;});
+          return platforms.indexOf(platform) !== -1 || platforms.indexOf('common') !== -1;
         })
         .sort();
       return commands;
@@ -125,11 +189,24 @@ function buildShortPagesIndex() {
       let reducer = (index, file) => {
         let os = utils.parsePlatform(file);
         let page = utils.parsePagename(file);
+        let language = utils.parseLanguage(file);
         if (index[page]) {
-          index[page].push(os);
+          let targets = index[page].targets;
+          let needsPush = true;
+          for (const target of targets) {
+            if (target.platform === os && target.language === language) {
+              needsPush = false;
+              continue;
+            }
+          }
+          if (needsPush) {
+            targets.push({'os': os, 'language': language});
+            index[page].targets = targets;
+          }
         } else {
-          index[page] = [os];
+          index[page] = {targets: [{'os': os, 'language': language}]};
         }
+
         return index;
       };
       return files.reduce(reducer, {});
@@ -142,7 +219,7 @@ function buildShortPagesIndex() {
 module.exports = {
   getShortIndex,
   hasPage,
-  findPlatform,
+  findPage,
   commands,
   commandsFor,
   clearPagesIndex,
diff --git a/lib/search.js b/lib/search.js
index 68b681e..1c85986 100644
--- a/lib/search.js
+++ b/lib/search.js
@@ -197,7 +197,7 @@ exports.printResults = (results, config) => {
     results.forEach((elem) => {
       let cmdname = utils.parsePagename(elem.file);
       let output = '    $ ' + cmdname;
-      let array = shortIndex[cmdname];
+      let array = shortIndex[cmdname]['platforms'];
       if (array.indexOf('common') === -1 && array.indexOf(preferredPlatform) === -1) {
         output += ' (Available on: ' + array.join(', ') + ')';
       }
diff --git a/lib/utils.js b/lib/utils.js
index cc2ebc8..4887305 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -6,13 +6,24 @@ const flatten = require('lodash/flatten');
 
 module.exports = {
   parsePlatform(pagefile) {
-    return path.dirname(pagefile);
+    const components = pagefile.split(path.sep);
+    return components[components.length-2];
   },
 
   parsePagename(pagefile) {
     return path.basename(pagefile, '.md');
   },
 
+  parseLanguage(pagefile) {
+    const components = pagefile.split(path.sep);
+    const langPathIndex = 3;
+    const langParts = components[components.length-langPathIndex].split('.');
+    if (langParts.length === 1) {
+      return 'en';
+    }
+    return langParts[1];
+  },
+
   isPage(file) {
     return path.extname(file) === '.md';
   },
@@ -34,7 +45,7 @@ module.exports = {
             if (stat.isDirectory()) {
               return walk(itemPath);
             }
-            return path.join(path.basename(dir), item);
+            return path.join(dir, item);
           });
         }));
       })


Test patch:
diff --git a/test/cache.spec.js b/test/cache.spec.js
index 11145bd..5b4e2ed 100644
--- a/test/cache.spec.js
+++ b/test/cache.spec.js
@@ -95,7 +95,7 @@ describe('Cache', () => {
     it('should return page contents for ls', () => {
       sinon.stub(fs, 'readFile').resolves('# ls\n> ls page');
       sinon.stub(platform, 'getPreferredPlatformFolder').returns('osx');
-      sinon.stub(index, 'findPlatform').resolves('osx');
+      sinon.stub(index, 'findPage').resolves('osx');
       const cache = new Cache(config.get());
       return cache.getPage('ls')
         .then((content) => {
@@ -103,28 +103,28 @@ describe('Cache', () => {
           content.should.startWith('# ls');
           fs.readFile.restore();
           platform.getPreferredPlatformFolder.restore();
-          index.findPlatform.restore();
+          index.findPage.restore();
         });
     });
 
     it('should return empty contents for svcs on OSX', () =>{
       sinon.stub(fs, 'readFile').resolves('# svcs\n> svcs');
       sinon.stub(platform, 'getPreferredPlatformFolder').returns('osx');
-      sinon.stub(index, 'findPlatform').resolves(null);
+      sinon.stub(index, 'findPage').resolves(null);
       const cache = new Cache(config.get());
       return cache.getPage('svc')
         .then((content) => {
           should.not.exist(content);
           fs.readFile.restore();
           platform.getPreferredPlatformFolder.restore();
-          index.findPlatform.restore();
+          index.findPage.restore();
         });
     });
 
     it('should return page contents for svcs on SunOS', () => {
       sinon.stub(fs, 'readFile').resolves('# svcs\n> svcs');
       sinon.stub(platform, 'getPreferredPlatformFolder').returns('sunos');
-      sinon.stub(index, 'findPlatform').resolves('svcs');
+      sinon.stub(index, 'findPage').resolves('svcs');
       const cache = new Cache(config.get());
       return cache.getPage('svcs')
         .then((content) => {
@@ -132,7 +132,7 @@ describe('Cache', () => {
           content.should.startWith('# svcs');
           fs.readFile.restore();
           platform.getPreferredPlatformFolder.restore();
-          index.findPlatform.restore();
+          index.findPage.restore();
         });
     });
 
diff --git a/test/functional-test.sh b/test/functional-test.sh
index 7d4ba09..84d88c3 100755
--- a/test/functional-test.sh
+++ b/test/functional-test.sh
@@ -10,8 +10,8 @@ function tldr-render-pages {
   tldr du --os=osx && \
   tldr du --os=linux --markdown && \
   tldr du --os=osx --markdown && \
-  tldr --random && \
-  tldr --random-example && \
+  LANG= tldr --random && \
+  LANG= tldr --random-example && \
   tldr --list && \
   tldr --list-all
 }
@@ -19,4 +19,6 @@ function tldr-render-pages {
 tldr --render $HOME/.tldr/cache/pages/common/ssh.md && \
 tldr --update && tldr-render-pages && \
 tldr --clear-cache && \
-tldr --update && tldr-render-pages
+tldr --update && tldr-render-pages && \
+LANG=pt_BR tldr-render-pages && \
+tldr --search "disk space"
diff --git a/test/index.spec.js b/test/index.spec.js
index c8eb134..a4143c1 100644
--- a/test/index.spec.js
+++ b/test/index.spec.js
@@ -7,20 +7,24 @@ const sinon = require('sinon');
 const should = require('should');
 
 const pages = [
-  'index.json',
-  'common/cp.md',
-  'common/git.md',
-  'common/ln.md',
-  'common/ls.md',
-  'linux/dd.md',
-  'linux/du.md',
-  'linux/top.md',
-  'osx/dd.md',
-  'osx/du.md',
-  'osx/top.md',
-  'sunos/dd.md',
-  'sunos/du.md',
-  'sunos/svcs.md'
+  '/index.json',
+  '/pages/linux/apk.md',
+  '/pages.zh/linux/apk.md',
+  '/pages/common/cp.md',
+  '/pages.it/common/cp.md',
+  '/pages.ta/common/cp.md',
+  '/pages/common/git.md',
+  '/pages/common/ln.md',
+  '/pages/common/ls.md',
+  '/pages/linux/dd.md',
+  '/pages/linux/du.md',
+  '/pages/linux/top.md',
+  '/pages/osx/dd.md',
+  '/pages/osx/du.md',
+  '/pages/osx/top.md',
+  '/pages/sunos/dd.md',
+  '/pages/sunos/du.md',
+  '/pages/sunos/svcs.md'
 ];
 
 describe('Index building', () => {
@@ -73,30 +77,86 @@ describe('Index', () => {
     fs.writeJson.restore();
   });
 
-  describe('findPlatform()', () => {
+  describe('findPage()', () => {
+    it('should find Linux platform for apk command for Chinese', () => {
+      return index.findPage('apk', 'linux', 'zh')
+        .then((folder) => {
+          return folder.should.equal('pages.zh/linux');
+        });
+    });
+
+    it('should find Linux platform for apk command for Chinese given Windows', () => {
+      return index.findPage('apk', 'windows', 'zh')
+        .then((folder) => {
+          return folder.should.equal('pages.zh/linux');
+        });
+    });
+
     it('should find Linux platform for dd command', () => {
-      return index.findPlatform('dd', 'linux')
+      return index.findPage('dd', 'linux', 'en')
         .then((folder) => {
-          return folder.should.equal('linux');
+          return folder.should.equal('pages/linux');
         });
     });
 
-    it('should find platform common for cp command', () => {
-      return index.findPlatform('cp', 'linux')
+    it('should find platform common for cp command for English', () => {
+      return index.findPage('cp', 'linux', 'en')
         .then((folder) => {
-          return folder.should.equal('common');
+          return folder.should.equal('pages/common');
         });
     });
 
-    it('should not find platform for svcs command on Linux', () => {
-      return index.findPlatform('svcs', 'linux')
+    it('should find platform common for cp command for Tamil', () => {
+      return index.findPage('cp', 'linux', 'ta')
         .then((folder) => {
-          return should.not.exist(folder);
+          return folder.should.equal('pages.ta/common');
+        });
+    });
+
+    it('should find platform common for cp command for Italian', () => {
+      return index.findPage('cp', 'linux', 'it')
+        .then((folder) => {
+          return folder.should.equal('pages.it/common');
+        });
+    });
+
+    it('should find platform common for cp command for Italian given Windows', () => {
+      return index.findPage('cp', 'windows', 'it')
+        .then((folder) => {
+          return folder.should.equal('pages.it/common');
+        });
+    });
+
+    it('should find platform common for ls command for Italian', () => {
+      return index.findPage('ls', 'linux', 'it')
+        .then((folder) => {
+          return folder.should.equal('pages/common');
+        });
+    });
+
+    it('should find platform common for cp command for Italian given common platform', () => {
+      return index.findPage('cp', 'common', 'it')
+        .then((folder) => {
+          return folder.should.equal('pages.it/common');
+        });
+    });
+
+    it('should find platform common for cp command for English given a bad language', () => {
+      return index.findPage('cp', 'linux', 'notexist')
+        .then((folder) => {
+          return folder.should.equal('pages/common');
+        });
+    });
+
+    it('should find platform for svcs command on Linux', () => {
+      return index.findPage('svcs', 'linux', 'en')
+        .then((folder) => {
+          return folder.should.equal('pages/sunos');
         });
     });
 
     it('should not find platform for non-existing command', () => {
-      return index.findPlatform('qwerty', 'linux')
+      return index.findPage('qwerty', 'linux', 'en')
         .then((folder) => {
           return should.not.exist(folder);
         });
@@ -107,7 +167,7 @@ describe('Index', () => {
     return index.commands()
       .then((commands) => {
         commands.should.deepEqual([
-          'cp', 'dd', 'du', 'git', 'ln', 'ls', 'svcs', 'top'
+          'apk', 'cp', 'dd', 'du', 'git', 'ln', 'ls', 'svcs', 'top'
         ]);
       });
   });
@@ -117,7 +177,7 @@ describe('Index', () => {
       return index.commandsFor('linux')
         .then((commands) => {
           commands.should.deepEqual([
-            'cp', 'dd', 'du', 'git', 'ln', 'ls', 'top'
+            'apk', 'cp', 'dd', 'du', 'git', 'ln', 'ls', 'top'
           ]);
         });
     });
@@ -145,14 +205,15 @@ describe('Index', () => {
     return index.getShortIndex()
       .then((idx) => {
         idx.should.deepEqual({
-          cp: ['common'],
-          git: ['common'],
-          ln: ['common'],
-          ls: ['common'],
-          dd: ['linux', 'osx', 'sunos'],
-          du: ['linux', 'osx', 'sunos'],
-          top: ['linux', 'osx'],
-          svcs: ['sunos']
+          apk: {targets: [{language: 'en', os: 'linux'}, {language: 'zh', os: 'linux'}]},
+          cp: {targets: [{language: 'en', os: 'common'}, {language: 'it', os: 'common'}, {language: 'ta', os: 'common'}]},
+          dd: {targets: [{language: 'en', os: 'linux'}, {language: 'en', os: 'osx'}, {language: 'en', os: 'sunos'}]},
+          du: {targets: [{language: 'en', os: 'linux'}, {language: 'en', os: 'osx'}, {language: 'en', os: 'sunos'}]},
+          git: {targets: [{language: 'en', os: 'common'}]},
+          ln: {targets: [{language: 'en', os: 'common'}]},
+          ls: {targets: [{language: 'en', os: 'common'}]},
+          svcs: {targets: [{language: 'en', os: 'sunos'}]},
+          top: {targets: [{language: 'en', os: 'linux'}, {language: 'en', os: 'osx'}]},
         });
       });
   });