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

add jsdocs to tldr and cache #355

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions lib/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,19 @@ class Cache {
this.cacheFolder = path.join(config.cache, 'cache');
}

/**
* Fetch stats from the cache folder.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Fetch stats from the cache folder.
* Fetch stats from the cache folder for getting its last modified time (mtime).

* @returns {Promise<any>} A promise with the stats of the cache folder.
*/
lastUpdated() {
return fs.stat(this.cacheFolder);
}

/**
* Fetch a page from cache using preferred language and preferred platform.
* @param page {} A
* @returns {Promise<unknown>}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between Promise<any> and Promise<unknown> ?

*/
getPage(page) {
let preferredPlatform = platform.getPreferredPlatformFolder(this.config);
const preferredLanguage = process.env.LANG || 'en';
Expand All @@ -36,10 +45,18 @@ class Cache {
});
}

/**
* Clean the cache folder.
* @returns {Promise<any>} A promise when the remove is completed.
*/
clear() {
return fs.remove(this.cacheFolder);
}

/**
* Update the cache folder and returns the English entries.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Update the cache folder and returns the English entries.
* Update the cache folder using a temporary directory, update the index and return it.

* @returns {Promise<any>} A promise with the index.
*/
update() {
// Temporary folder path: /tmp/tldr/{randomName}
const tempFolder = path.join(os.tmpdir(), 'tldr', utils.uniqueId());
Expand Down
50 changes: 39 additions & 11 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,24 @@ function hasLang(targets, 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.
/**
* Check if a page is in the index.
* @returns {boolean} A boolean that indicates if the page is present.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {boolean} A boolean that indicates if the page is present.
* @returns {boolean} The presence of the page in the index.

*/
function hasPage(page) {
// 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.
if (!shortIndex) {
return false;
}
return page in shortIndex;
}

// Return all commands available in the local cache.
/**
* Return all commands available in the local cache.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Return all commands available in the local cache.
* Return all commands available in the local index.

* @returns {Promise<string[]>} A promise with the commands from cache.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {Promise<string[]>} A promise with the commands from cache.
* @returns {Promise<string[]>} A promise with the commands from the index.

*/
function commands() {
return getShortIndex().then((idx) => {
return Object.keys(idx).sort();
Expand All @@ -110,6 +117,13 @@ function commands() {

// Return all commands for a given platform.
// P.S. - The platform 'common' is always included.
Comment on lines 118 to 119
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove this then? Seems repeated.

/**
* Return all commands for a given platform.
*
* The 'common' platform is always included.
* @param platform {string} The platform.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param platform {string} The platform.
* @param platform {string} The desired platform.

* @returns {Promise<string[]>} A promise with the commands for a given platform.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {Promise<string[]>} A promise with the commands for a given platform.
* @returns {Promise<string[]>} The commands for a given platform.

I don't think we need to specify that it is a promise again

*/
function commandsFor(platform) {
return getShortIndex()
.then((idx) => {
Expand All @@ -124,7 +138,11 @@ function commandsFor(platform) {
});
}

// Delete the index file.
/**
* Delete the index file.
*
* @returns {Promise<any>} A promise when the remove is completed.
*/
function clearPagesIndex() {
return fs.unlink(shortIndexFile)
.then(() => {
Expand All @@ -139,7 +157,9 @@ function clearPagesIndex() {
});
}

// Set the shortIndex variable to null.
/**
* Set the shortIndex variable to null.
*/
function clearRuntimeIndex() {
shortIndex = null;
}
Expand All @@ -150,18 +170,26 @@ function rebuildPagesIndex() {
});
}

// If the variable is not set, read the file and set it.
// Else, just return the variable.
/**
* Return the index.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Return the index.
* Return the index, that contains all available commands with their target os and platform.

*
* If the index is not loaded, read the file and load it.
* @returns {Promise<any>} The index entries.
*/
function getShortIndex() {
if (shortIndex) {
return Promise.resolve(shortIndex);
}
return readShortPagesIndex();
}

// Read the index file, and load it into memory.
// If the file does not exist, create the data structure, write the file,
// and load it into memory.
/**
* Read the index file, and load it into memory.
*
* If the file does not exist, create the data structure, write the file,
* and load it into memory.
* @returns {Promise<any>} The index entries.
*/
function readShortPagesIndex() {
return fs.readJson(shortIndexFile)
.then((idx) => {
Expand Down
6 changes: 5 additions & 1 deletion lib/remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ const unzip = require('node-unzip-2');
const config = require('./config');
const axios = require('axios');

// Downloads the zip file from github and extracts it to folder
/**
* Download the zip file from GitHub and extract it to folder.
* @param path {string} Path to destination folder.
* @returns {Promise<void>} A promise when the operation is completed.
*/
exports.download = (path) => {
const url = config.get().repository;

Expand Down
80 changes: 78 additions & 2 deletions lib/tldr.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ class Tldr {
this.cache = new Cache(this.config);
}

/**
* Print pages for a given platform..
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple periods at the end of the sentence.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Print pages for a given platform..
* Print pages for a given platform.

* @param singleColumn {boolean} A boolean to print one command per line.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param singleColumn {boolean} A boolean to print one command per line.
* @param singleColumn {boolean} Print one command per line.

* @returns {Promise<void>} A promise when the operation is completed.
*/
list(singleColumn) {
let os = platform.getPreferredPlatformFolder(this.config);
return index.commandsFor(os)
Expand All @@ -29,17 +34,33 @@ class Tldr {
});
}

/**
* Print all pages in the cache.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Print all pages in the cache.
* Print the name of all pages in the index.

* @param singleColumn {boolean} A boolean to print one command per line.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param singleColumn {boolean} A boolean to print one command per line.
* @param singleColumn {boolean} Print one command per line.

IMO "a boolean to" is redundant.

* @returns {Promise<void>} A promise when the operation is completed.
*/
listAll(singleColumn) {
return index.commands()
.then((commands) => {
return this.printPages(commands, singleColumn);
});
}

/**
* Print a command page.
* @param commands {string[]} A given command to be printed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"A given command" is incorrect when we are passing an array. Let's change this to plural.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param commands {string[]} A given command to be printed.
* @param commands {string[]} Given commands to be printed.

* @param options {object} The options for the render.
* @returns {Promise<void>} A promise when the operation is completed.
*/
get(commands, options) {
return this.printBestPage(commands.join('-'), options);
}

/**
* Print a random page for the current platform.
* @param options {object} The options for the render.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param options {object} The options for the render.
* @param options {Object} The options for the render.
* @param options.markdown {boolean} Print the markdown as-is.
* @param options.randomExample {boolean} Print a random example of the page.

* @returns {Promise<void>} A promise when the operation is completed.
*/
random(options) {
let os = platform.getPreferredPlatformFolder(this.config);
return index.commandsFor(os)
Expand All @@ -56,6 +77,10 @@ class Tldr {
});
}

/**
* Print a random page.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Print a random page.
* Print a random page example.

* @returns {Promise<void>} A promise when the opration is completed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {Promise<void>} A promise when the opration is completed.
* @returns {Promise<void>} A promise when the operation is completed.

*/
randomExample() {
let os = platform.getPreferredPlatformFolder(this.config);
return index.commandsFor(os)
Expand All @@ -72,6 +97,11 @@ class Tldr {
});
}

/**
* Print a markdown file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Print a markdown file.
* Render a markdown file.

* @param file {string} The path to the file.
* @returns {Promise<void>} A promise when the operation is completed.
*/
render(file) {
return fs.readFile(file, 'utf8')
.then((content) => {
Expand All @@ -82,24 +112,41 @@ class Tldr {
});
}

/**
* Clear the cache folder.
* @returns {Promise<void>} A promise when the cache is deleted.
*/
clearCache() {
return this.cache.clear().then(() => {
console.log('Done');
});
}

/**
* Update the cache.
* @returns {Promise<any>} A promise with the index.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "A promise with the cache"?

*/
updateCache() {
return spinningPromise('Updating...', () => {
return this.cache.update();
});
}

/**
* Update the index.
* @returns {Promise<any>} A promise with the index.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {Promise<any>} A promise with the index.
* @returns {Promise<any>} The index.

*/
updateIndex() {
return spinningPromise('Creating index...', () => {
return search.createIndex();
});
}

/**
* Search some keywords in the index and print the results.
* @param keywords {string[]} The given keywords.
* @returns {Promise<any>} A promise when the operation is completed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {Promise<any>} A promise when the operation is completed.
* @returns {Promise<any>} A promise when the operation is completed.

This promise doesn't have any value.

*/
search(keywords) {
return search.getResults(keywords.join(' '))
.then((results) => {
Expand All @@ -108,6 +155,12 @@ class Tldr {
});
}

/**
* Print all pages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Print all pages.
* Print the name of all pages.

* @param pages {string[]} A list of pages to be printed.
* @param singleColumn {boolean} A boolean to print one command per line.
* @returns {Promise<void>} A promise when the operation is completed.
*/
printPages(pages, singleColumn) {
if (pages.length === 0) {
throw new EmptyCacheError();
Expand All @@ -120,7 +173,15 @@ class Tldr {
});
}

printBestPage(command, options={}) {
/**
* Print a page from the cache.
*
* If the page is not present, the cache is updated.
* @param command {string} The given command to be printed.
* @param options {object} The options for the render.
* @returns {Promise<void>} A promise when the operation is completed.
*/
printBestPage(command, options= {}) {
// Trying to get the page from cache first
return this.cache.getPage(command)
.then((content) => {
Expand Down Expand Up @@ -153,6 +214,10 @@ class Tldr {
});
}

/**
* Print a warning message if the cache is 30 days old.
* @returns {Promise<void>} A promise when the operation is completed.
*/
checkStale() {
return this.cache.lastUpdated()
.then((stats) => {
Expand All @@ -162,7 +227,12 @@ class Tldr {
});
}

renderContent(content, options={}) {
/**
* Print the page content.
* @param content {string} The content of a page.
* @param options {object<{markdown: boolean, randomExample: boolean}>} The options for the render.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param options {object<{markdown: boolean, randomExample: boolean}>} The options for the render.
* @param options {Object} The options for the render.
* @param options.markdown {boolean} Print the markdown as-is.
* @param options.randomExample {boolean} Print a random example of the page.

*/
renderContent(content, options= {}) {
if (options.markdown) {
return console.log(content);
}
Expand All @@ -177,6 +247,12 @@ class Tldr {
}
}

/**
* Display a spinner while a task is running.
* @param text {string} The text of the spinner.
* @param factory {Function} A task to be run.
* @returns {Promise} A promise with the result of the task.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {Promise} A promise with the result of the task.
* @returns {Promise<any>} A promise with the result of the task.

Any result can be returned here, because any function (task) can be passed.

*/
function spinningPromise(text, factory) {
const spinner = ora();
spinner.start(text);
Expand Down