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

Fix issue where common won't be required if os is detected #50

Merged
merged 1 commit into from
Jan 29, 2014

Conversation

Tug
Copy link
Contributor

@Tug Tug commented Jan 28, 2014

Erratum in last pull request #44
Must specify default platform "common" otherwise it will not be requested.

@rprieto
Copy link
Contributor

rprieto commented Jan 28, 2014

My bad, I should have tested it more thoroughly...
Thanks for the fix!

@Tug Tug closed this Jan 28, 2014
@Tug Tug reopened this Jan 28, 2014
@@ -14,7 +13,7 @@ function path(command, platform) {
}

function getForPlatform(repository, command, platform, done) {
var pathCommon = path(command);
var pathCommon = path(command, "common");
var pathPlatform = path(command, platform);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we expect platform to be set? If so would it just skip the OS lookup?

function path(command, platform) {
  return (platform || osDirectories[os.platform()]) + '/' + command + '.md';
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I added the optimist module so you can pass --os=osx as parameter to have the documentation for Mac OS X. This option is set as parameter to path for pathPlatform if it exists. It's more for debugging purpose though.

rprieto added a commit that referenced this pull request Jan 29, 2014
Fix issue where common won't be required if os is detected
@rprieto rprieto merged commit 3aafb4f into tldr-pages:master Jan 29, 2014
@waldyrious waldyrious added the architecture Organization of the pages per language, platform, etc. label Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture Organization of the pages per language, platform, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants