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

docs: incorrect man page link in html modules doc #5686

Closed
sbc100 opened this issue Mar 13, 2016 · 12 comments
Closed

docs: incorrect man page link in html modules doc #5686

sbc100 opened this issue Mar 13, 2016 · 12 comments
Labels
doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.

Comments

@sbc100
Copy link

sbc100 commented Mar 13, 2016

In https://nodejs.org/dist/latest-v5.x/docs/api/modules.html#modules_modules

The following line appears:
var mySquare = square(2);

Which should just be:
var mySquare = square(2);

Looks like the thing that generates links to man pages automatically is a little overzealous.

@claudiorodriguez claudiorodriguez added doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem. labels Mar 13, 2016
@jvcjunior
Copy link

Hi, I am new in the open source world and would like to help with this. I think it is easy and it would be my second experience contributing to an open source project. Can I do this?

@Fishrock123 Fishrock123 added the good first issue Issues that are suitable for first-time contributors. label Mar 14, 2016
@Fishrock123
Copy link
Contributor

@jvcjunior Sure, It's some regex parsing. The code can be found at https://github.com/nodejs/node/blob/master/tools/doc/html.js#L183-L198

It's possible that we should instead put some extra formatting around what should be man links, something like [man(2)]() maybe?

cc @nodejs/documentation for advice

@claudiorodriguez claudiorodriguez added tools Issues and PRs related to the tools directory. and removed module Issues and PRs related to the module subsystem. labels Mar 14, 2016
@claudiorodriguez
Copy link
Contributor

We could either add some extra formatting (which would require changing all man links in all the docs, not sure how many there are), or change the doc markdown-to-html tool so that linkManPages is not called within code blocks (by tracking tok.type, sort of like parseLists does). That wouldn't solve the problem in a definitive fashion of course, it just changes less files, and the two can be done in parallel, even. It mostly depends on how many man links there are lying around. If there ain't that many, I'd say go for changing the format, and change the regex in the tool and the links in the markdown (you'll need to rebase the PR periodically).

@mithun-daa
Copy link
Contributor

Just out of curiosity, how do I convert the mardown files to html locally?

@Fishrock123
Copy link
Contributor

@mithun-daa make doc (assuming you are on Unix and have make)

See https://github.com/nodejs/node/blob/master/Makefile#L238-L249 for the source, or if on windows

@mithun-daa
Copy link
Contributor

Another way to solve it would be not to convert to links (linkManPages) when the tok.type === code.

function parseText(lexed) {
  lexed.forEach(function(tok) {
    if (tok.text && tok.type !== 'code') {
      tok.text = linkManPages(tok.text);
    }
  });
}

@Fishrock123 Fishrock123 removed the good first issue Issues that are suitable for first-time contributors. label Mar 15, 2016
@Fishrock123
Copy link
Contributor

(Removing label so that @jvcjunior can tackle this)

@mithun-daa Hmm, what if it were in a code comment?

@mithun-daa
Copy link
Contributor

@Fishrock123 Can code blocks have man page links? If I am not mistaken you cannot have any other markdown formatting inside a code block. I tried doing the same on some online markdown editors and it does not work.

@Fishrock123
Copy link
Contributor

I wasn't sure, but fair enough. :)

@mithun-daa
Copy link
Contributor

@jvcjunior This should be a straight enough fix. Let me know if you can do it. If not I'll send in a PR.

@jvcjunior
Copy link

@mithun-daa Don't worry about me. You can do this. No problem at all. ;)

mithun-daa added a commit to mithun-daa/node that referenced this issue Mar 15, 2016
Do not call the linkManPages if the tok type is code
@mithun-daa
Copy link
Contributor

Sent PR

MylesBorins pushed a commit that referenced this issue Mar 17, 2016
Do not call the linkManPages if the tok type is code

Fixes: #5686
PR-URL: #5721
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 21, 2016
Do not call the linkManPages if the tok type is code

Fixes: #5686
PR-URL: #5721
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 21, 2016
Do not call the linkManPages if the tok type is code

Fixes: #5686
PR-URL: #5721
Reviewed-By: James M Snell <[email protected]>
Fishrock123 pushed a commit that referenced this issue Mar 22, 2016
Do not call the linkManPages if the tok type is code

Fixes: #5686
PR-URL: #5721
Reviewed-By: James M Snell <[email protected]>
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

No branches or pull requests

5 participants