-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
main: add "Other versions" links to frontmatter of HTML-rendered specs #4401
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks!
@earth2marsh Forgot to adjust the unit tests 🙄 |
95d128c
to
70a5c94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
@@ -53,7 +55,7 @@ for specification in $specifications; do | |||
|
|||
echo === Building $version to $destination | |||
|
|||
node scripts/md2html/md2html.js --maintainers $maintainers $specification > $tempfile | |||
node scripts/md2html/md2html.js --maintainers $maintainers $specification "$allVersions" > $tempfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a yargs array, rather than overloading the positional argument.
diff --git a/scripts/md2html/build.sh b/scripts/md2html/build.sh
index b40a1a12..c0ef1654 100755
--- a/scripts/md2html/build.sh
+++ b/scripts/md2html/build.sh
@@ -55,7 +55,11 @@ for specification in $specifications; do
echo === Building $version to $destination
- node scripts/md2html/md2html.js --maintainers $maintainers $specification "$allVersions" > $tempfile
+ node scripts/md2html/md2html.js \
+ --maintainers $maintainers \
+ --all-versions $allVersions -- \
+ $specification > $tempfile
+
npx respec --no-sandbox --use-local --src $tempfile --out $destination
rm $tempfile
diff --git a/scripts/md2html/md2html.js b/scripts/md2html/md2html.js
index c3ff2862..2873b3d4 100644
--- a/scripts/md2html/md2html.js
+++ b/scripts/md2html/md2html.js
@@ -55,6 +55,9 @@ let argv = require('yargs')
.alias('m','maintainers')
.describe('maintainers','path to MAINTAINERS.md')
.demandCommand(1)
+ .array('all-versions')
+ .alias('av', 'all-versions')
+ .describe('all-versions', 'List of all versions of the specification, used for inter-version linking')
.argv;
const abstract = 'What is the OpenAPI Specification?';
let maintainers = [];
@@ -77,7 +80,7 @@ const md = require('markdown-it')({
});
function preface(title,options) {
- const otherVersions = options._[1].split("\n").map(v => path.basename(v,'.md')).filter(v => v !== options.subtitle);
+ const otherVersions = options['all-versions'].map(v => path.basename(v,'.md')).filter(v => v !== options.subtitle)
const respec = {
specStatus: "base",
latestVersion: "https://spec.openapis.org/oas/latest.html",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion.
If the command-line interface of md2html were to survive, an array of positional parameters could be seen as more elegant, and preferable to a dash-dash option because allVersions is not optional.
In #4234 we plan to move the whole md2html stuff to a separate repository and reshape it into a GitHub action, which will remove the current yargs solution along with all bash parts and replace it with a JS function md2html(spec,editors,formerEditors,allVersions)
or similar. So I don't want to invest into the current command-line interface.
Merged as the current state is what @earth2marsh previously approved plus the repaired unit tests. |
This PR resolves a Slack discussion started by @earth2marsh:
It includes a list of "Other versions" links to the frontmatter of the HTML-rendered specs, for example
Tick one of the following options: