-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: link to man pages #5073
doc: link to man pages #5073
Conversation
Links to e.g. http://man7.org/linux/man-pages/man2/chmod.2.html |
// '<a href="http://man7.org/linux/man-pages/man2/open.2.html">open(2)</a>' | ||
function getManLink(text) { | ||
// It's safe to concatenate into HTML like this because `text` matches | ||
var found = text.match(/([a-z]+)\(([0-9])\)/); |
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.
Only one digit inside the parenthesis? There could be more than one. Also, use \d
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.
I think man pages only go 1-8 actually
http://unix.stackexchange.com/questions/3586/what-do-the-numbers-in-a-man-page-mean
Changed it to use \d
// Handle references to man pages, eg "open(2)" or "lchmod(2)" | ||
// Returns modified text, with such refs replace with HTML links | ||
function linkManPages(text) { | ||
return text.replace(/ [a-z]+\([0-9]\)/, getManLink); |
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.
Instead of matching again in getManLink
, grouping can be done here itself, right?
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.
Also what if there is more than one reference?
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.
yeah good call, i changed it to /g
i did the grouping there to make it clear that name
and number
are safe to concat into an HTML string, but i'll just combine the two methods to keep things simple
/cc @nodejs/documentation |
7bc5a9d
to
445a1e4
Compare
Would also be good to handle cases when POSIX call is wrapped in backquotes. For, example in #5075 I did this with a |
@@ -165,10 +175,32 @@ function parseLists(input) { | |||
} | |||
|
|||
|
|||
// Syscalls which appear in the docs, but which only exist in BSD / OSX | |||
var BSD_ONLY_SYSCALLS = ['lchmod']; |
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.
Suggestion: Now that we have Set
s in core, we may be able to speed up this lookup.
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.
Yeah, I would think that utilizing a Set
or even an object would be faster here (assuming we get more items added to here as well.
var BSD_ONLY_SYSCALLS = new Set(['lchmod']);
...
if (BSD_ONLY_SYSCALLS.has(name)( {
...
} else {
...
}
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.
done
I think every link should also have a [?] ( |
'&sektion=' + number + '">' + displayAs + '</a>'; | ||
} else { | ||
return ' <a href="http://man7.org/linux/man-pages/man' + number + '/' + | ||
name + '.' + number + '.html">' + displayAs + '</a>'; |
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.
cc @bnoordhuis what so you think about these sources for generic man docs?
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.
I was curious about that as well. I went through some digging on Man pages, so that particular set of docs is linked to from kernel.org as well. There's a bunch more sources for the Linux docs but I can't really find any official source. die.net for example.
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.
freebsd.org and man7.org are the official man pages so +1 from me. For freebsd.org, consider linking to the https:// site.
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.
done
@Fishrock123 dunno, some parts of the docs, like here, have a lot of man page links. I think it would look distracting for each of those to have a superscript. |
Hmmm, then we should probably add a thing to "about these docs" at the very least. |
// Returns modified text, with such refs replace with HTML links, for example | ||
// '<a href="http://man7.org/linux/man-pages/man2/open.2.html">open(2)</a>' | ||
function linkManPages(text) { | ||
return text.replace(/ ([a-z]+)\((\d)\)/gm, function(match, name, number) { |
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.
Would the regex match if the markdown had a code snippet with, say, foo(1)
in it?
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.
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.
NVM. Here's a place where it fails
doc/api/modules.markdown:var mySquare = square(2);
I'll change it to require special markup to generate the link, so we don't get false positives.
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.
@bnoordhuis actually it might be fine!
I just generated the docs and it seems that JS passes through the HTML syntax highlighter before it gets to linkManPages
. The square(2)
above doesn't turn into a link.
@Fishrock123 done |
|
||
**Caveat:** some syscalls, like lchown(2), are BSD-specific. That means, for example, that `fs.lchown()` only works on Mac OSX and other BSD-derived systems, and is not available on Linux. | ||
|
||
Most Unix syscalls have Windows equivalents, but behavior may differ on Windows relative to Linux and OSX. For an example of the subtle ways in which it's sometimes impossible to replace Unix syscall semantics on Windows, see [Node issue 4760](https://github.com/nodejs/node/issues/4760). |
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.
Can you wrap lines at 80 columns in this file?
LGTM with style nits. |
LGTM with @bnoordhuis' nits addressed |
@bnoordhuis @jasnell fixed LMK if you want me to squash this PR to a single commit |
LGTM |
@@ -66,3 +66,19 @@ Every HTML file in the markdown has a corresponding JSON file with the | |||
same data. |
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.
@estliberitas Hope you are taking care of this page as well :)
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.
@thefourtheye Np, will do soon )
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 :)
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.
@thefourtheye Hi, I'm a bit out of context. I'll do the usual stuff I do for API pages. Also, once I structure what's on my mind, I'll add maybe a section to this page on how things should be formatted, etc. If it's not added yet, hah.
Sounds OK? Or I do not get what should be done, coz the only thing to be added here would be something like a cool guide on formatting the stuff. But I'm not sure if this page is appropriate for this.
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.
Ah my bad. I just saw few linkable text on this page and reached out for your help. Perhaps we can leave this page as nothing much is needed here.
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.
Anyways, I'll continue my crusade on docs today and tomorrow. :)
On Sat, Feb 13, 2016, 08:59 thefourtheye [email protected] wrote:
In doc/api/documentation.markdown
#5073 (comment):@@ -66,3 +66,19 @@ Every HTML file in the markdown has a corresponding JSON file with the
same data.Ah my bad. I just saw few linkable text on this page and reached out for
your help. Perhaps we can leave this page as nothing much is needed here.—
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/pull/5073/files#r52824032.Sincerely,
Alexander Makarenko
LGTM |
@dcposch please rebase you branch. |
@dcposch if you could use a hand squashing your commits into one, please let me know |
@a0viedo @silverwind done |
Regarding the section |
pages (short for manual pages) which describe how the syscalls work. | ||
|
||
**Caveat:** some syscalls, like lchown(2), are BSD-specific. That means, for | ||
example, that `fs.lchown()` only works on Mac OSX and other BSD-derived systems, |
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.
Tiny nit: we write Mac OS X
(space before X) almost everywhere else.
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.
@bnoordhuis fixed
Changes LGTM sans nit. If other collaborators agree, please land it. |
This changes the doc generator to automatically link references such as `open(2)` to a man page on man7.org or freebsd.org
@silverwind dunno, syscalls are used elsewhere to (not just in |
Still LGTM |
Fix missing links. Fix styling of printf() - once #5073 lands, link to man page will be auto-generated. Fix several typos. PR-URL: #5225 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]>
@dcposch hmm, alright then. LGTM. |
Fix missing links. Fix styling of printf() - once #5073 lands, link to man page will be auto-generated. Fix several typos. PR-URL: #5225 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]>
'&sektion=' + number + '">' + displayAs + '</a>'; | ||
} else { | ||
return ' <a href="http://man7.org/linux/man-pages/man' + number + | ||
'/' + name + '.' + number + '.html">' + displayAs + '</a>'; |
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.
template literal?
Fix missing links. Fix styling of printf() - once #5073 lands, link to man page will be auto-generated. Fix several typos. PR-URL: #5225 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fix missing links. Fix styling of printf() - once #5073 lands, link to man page will be auto-generated. Fix several typos. PR-URL: #5225 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]>
Thanks! Landed in 4e77a7c. |
This changes the doc generator to automatically link references such as `open(2)` to a man page on man7.org or freebsd.org PR-URL: #5073 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
This changes the doc generator to automatically link references such as `open(2)` to a man page on man7.org or freebsd.org PR-URL: #5073 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
This changes the doc generator to automatically link references such as `open(2)` to a man page on man7.org or freebsd.org PR-URL: #5073 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
This changes the doc generator to automatically link references such as `open(2)` to a man page on man7.org or freebsd.org PR-URL: #5073 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
Fix missing links. Fix styling of printf() - once #5073 lands, link to man page will be auto-generated. Fix several typos. PR-URL: #5225 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]>
This changes the doc generator to automatically link references such as `open(2)` to a man page on man7.org or freebsd.org PR-URL: #5073 Reviewed-By: Ben Noorhduis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
This changes the doc generator to automatically link references such as
open(2)
to a man page on man7.org or freebsd.orgFixes #4375