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 script to automatically dump CLI usage into documentation #3589

Merged
merged 1 commit into from Nov 30, 2018

Conversation

boneskull
Copy link
Member

@boneskull boneskull commented Nov 29, 2018

this looks for <!-- usage --> and <!-- usagestop --> in docs/index.md and replaces everything between these two comments with the current output of bin/mocha --help.

@boneskull boneskull added the area: documentation anything involving docs or mochajs.org label Nov 29, 2018

Commands:
init <path> initialize a client-side mocha setup at <path>
init <path> initialize a client-side mocha setup at <path>
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was indentation removed? Running --help on most programs displays cmdline options with indentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

probably has to do with the lack of a tty somewhere.

the actual bin/mocha --help output has indents.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's what I thought this was supposed to do -- run mocha --help and paste its exact content.

Copy link
Member Author

Choose a reason for hiding this comment

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

it does, but the exact content when run without a terminal window doesn't have the leading indents, ostensibly because process.stdout.columns is undefined.

I don't know how to force this value to be present unless I monkeypatch it; it comes out of Node.js internals. it'd need to be monkeypatched for this script only, which is honestly more effort than I'd like to go to... if you can figure out how to fake a tty for this, go ahead and make the necessary changes. I'm curious about the solution, anyway!

@boneskull boneskull force-pushed the auto-doc-usage branch 2 times, most recently from c3d90a1 to 80e45a3 Compare November 29, 2018 21:44
@coveralls
Copy link

coveralls commented Nov 29, 2018

Coverage Status

Coverage remained the same at 90.495% when pulling bded805 on auto-doc-usage into 9f02180 on master.

}

if (sections.length === 3) {
// this is triggered if we already have stuff in between the comments
Copy link
Contributor

Choose a reason for hiding this comment

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

So if a usage codefence is already present? When would that not be the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, meaning, if we have

<!-- usage -->
STUFF
<!-- usagestop -->

we hit the first branch, otherwise if we have this (whitespace or nothing):

<!-- usage -->

<!-- usagestop -->

we hit the second.

Copy link
Contributor

Choose a reason for hiding this comment

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

no, meaning, if we have

<!-- usage -->
STUFF
<!-- usagestop -->

we hit the first branch, otherwise if we have this (whitespace or nothing):

<!-- usage -->

<!-- usagestop -->

we hit the second.

Well, that was what I assumed, where STUFF=code fence. Just not following when the second case would ever occur.

- rename `scripts/docs-update-toc.js` to reflect that it does more than just that
@boneskull boneskull added the semver-patch implementation requires increase of "patch" version number; "bug fixes" label Nov 30, 2018
@boneskull boneskull added this to the v6.0.0 milestone Nov 30, 2018
@boneskull
Copy link
Member Author

merging this; if there are further concerns, please send edits

@boneskull boneskull merged commit 955a71e into master Nov 30, 2018
@boneskull boneskull deleted the auto-doc-usage branch November 30, 2018 19:21
Copy link
Contributor

@plroebuck plroebuck left a comment

Choose a reason for hiding this comment

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

Well, guess it doesn't matter since it's already merged...

@@ -512,6 +513,7 @@
"markdownlint-cli": "^0.9.0",
"nps": "^5.7.1",
"nyc": "^11.7.3",
"mississippi": "^3.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Move "mississippi" to L513.5

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not done alphabetically?

}

if (sections.length === 3) {
// this is triggered if we already have stuff in between the comments
Copy link
Contributor

Choose a reason for hiding this comment

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

no, meaning, if we have

<!-- usage -->
STUFF
<!-- usagestop -->

we hit the first branch, otherwise if we have this (whitespace or nothing):

<!-- usage -->

<!-- usagestop -->

we hit the second.

Well, that was what I assumed, where STUFF=code fence. Just not following when the second case would ever occur.

});
}

updateTOC().then(updateUsage());
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need a catch here?

updateTOC().then(updateUsage()).catch((err) => {
  const script = path.basename(
    require.main.filename, path.extname(require.main.filename)
  );
  console.error(`${script}:`, err);
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation anything involving docs or mochajs.org semver-patch implementation requires increase of "patch" version number; "bug fixes"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants