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

tools: make sure doctool anchors respect @include #6943

Closed

Conversation

addaleax
Copy link
Member

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

doctool

Description of change

Previously, output files which were created using @include (notably, the single-page all.html) had basically broken internal links all over the place because references like errors.html#errors_class_error are being used, yet id attributes were generated that looked like
all_class_error.

This PR adds generation of comments from the @include preprocessor that indicate from which file the current markdown bits come and lets the HTML output generation take advantage of that so that more appropriate id attributes can be generated.

@addaleax addaleax added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels May 23, 2016
@MylesBorins
Copy link
Contributor

/cc @nodejs/documentation

@eljefedelrodeodeljefe
Copy link
Contributor

Rubber stamp LGTM

@addaleax
Copy link
Member Author

@eljefedelrodeodeljefe This contains actual code changes in the doctool, so I’d be glad if you’d take a quick look at the implementation when reviewing :)

@ChALkeR
Copy link
Member

ChALkeR commented May 24, 2016

Not sure if this should include a mention of a GitHub user in the commit message.

@addaleax addaleax force-pushed the doc-fix-broken-refs-singlepage branch from 514aab9 to 6f5e74e Compare May 24, 2016 10:40
@addaleax
Copy link
Member Author

@ChALkeR ooooh… good point, fixed

@addaleax
Copy link
Member Author

@addaleax
Copy link
Member Author

Also /cc @firedfox maybe?

@firedfox
Copy link
Contributor

LGTM

Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.
@addaleax addaleax force-pushed the doc-fix-broken-refs-singlepage branch from 6f5e74e to e345dbb Compare May 25, 2016 20:57
@addaleax
Copy link
Member Author

addaleax commented May 25, 2016

Rebased, only conflict was a rather expected one in test/doctool/test-doctool-html.js.

CI again to be on the safe side: https://ci.nodejs.org/job/node-test-commit/3498/ again² because of general CI weirdness: https://ci.nodejs.org/job/node-test-commit/3501/

addaleax added a commit that referenced this pull request May 25, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: #6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
@addaleax
Copy link
Member Author

Landed in b140bad

@eljefedelrodeodeljefe Happy rebasing ;)

@addaleax addaleax closed this May 25, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 30, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: nodejs#6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: #6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
@MylesBorins
Copy link
Contributor

@addaleax lts?

@addaleax addaleax deleted the doc-fix-broken-refs-singlepage branch July 11, 2016 22:19
@MylesBorins
Copy link
Contributor

@addaleax this one does not at all land cleanly :S

addaleax added a commit to addaleax/node that referenced this pull request Jul 12, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: nodejs#6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: #6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: #6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: #6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: #6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants