FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: fix relative links in net docs by evanlucas · Pull Request #5358 · nodejs/node · GitHub

/ node Public

doc: fix relative links in net docs - #5358

Closed
evanlucas wants to merge 1 commit into
nodejs:masterfrom
evanlucas:netlinks
ConversationCommits (1)ChecksFiles changed
Closed

doc: fix relative links in net docs#5358
evanlucas wants to merge 1 commit into
nodejs:masterfrom
evanlucas:netlinks

Conversation

evanlucas commented Feb 22, 2016

Copy link
Copy Markdown
Contributor

They were not being processed as relative links. Marked apparently does not like square brackets inside of relative links.

doc: fix relative links in net docs
fad1295 
They were not being processed as relative links.
evanlucas added the doc Issues and PRs related to the documentations. label Feb 22, 2016
evanlucas mentioned this pull request Feb 22, 2016
Closed
mscdex added the net Issues and PRs related to the net subsystem. label Feb 22, 2016

evanlucas commented Feb 24, 2016

Copy link
Copy Markdown
Contributor Author

/cc @nodejs/documentation

silverwind commented Feb 25, 2016

Copy link
Copy Markdown
Contributor

LGTM.

Looks like this was in the tree for quite some time. Did we switch the Markdown parser recently, or why is this being noticed now?

thefourtheye reviewed Feb 25, 2016
View reviewed changes
Comment thread doc/api/net.markdown
[`server.getConnections()`]: #net_server_getconnections_callback
[`server.listen(port, \[host\], \[backlog\], \[callback\])`]: #net_server_listen_port_hostname_backlog_callback
[`socket.connect(options\[, connectListener\])`]: #net_socket_connect_options_connectlistener
[`server.listen(port, host, backlog, callback)`]: #net_server_listen_port_hostname_backlog_callback

thefourtheye Feb 25, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can't this be simply server.listen?

silverwind Feb 25, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think so, the link signature of these methods include the arguments. How are these generated, by the way?

thefourtheye Feb 25, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@silverwind You mean, this tag could have been auto-generated with a tool?

evanlucas Feb 25, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think they are generated

silverwind Feb 25, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The <a id>'s which these link to are certainly autogenerated from the doctool:

<h3>
  server.listen(port[, hostname][, backlog][, callback])
  <span>
    <a class="mark" href="#net_server_listen_port_hostname_backlog_callback" id="net_server_listen_port_hostname_backlog_callback">#
    </a>
  </span>
</h3>

They long ids are necessary to link to different method signatures, and there can only be one id per element in HTML, so I think your suggestion is not possible.

silverwind Feb 25, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oh, and I just notice that I misread your suggestion completely, please disregard me :)

silverwind commented Feb 27, 2016

Copy link
Copy Markdown
Contributor

Ping @evanlucas, I'd also be in favor of shortening these link names to just server.listen.

evanlucas commented Feb 27, 2016

Copy link
Copy Markdown
Contributor Author

Yea, the reason I didn't do that is because there are multiple server.listen headings. That is why the arguments are included in the link

silverwind commented Feb 27, 2016

Copy link
Copy Markdown
Contributor

Alright, landed in 1c30d60.

silverwind closed this Feb 27, 2016
silverwind pushed a commit that referenced this pull request Feb 27, 2016
doc: fix relative links in net docs
1c30d60 
They were not being processed as relative links.

PR-URL: #5358
Reviewed-By: Roman Reiss <me@silverwind.io>
rvagg pushed a commit that referenced this pull request Feb 28, 2016
doc: fix relative links in net docs
534e88f 
They were not being processed as relative links.

PR-URL: #5358
Reviewed-By: Roman Reiss <me@silverwind.io>
Fishrock123 mentioned this pull request Mar 1, 2016
Merged
5 tasks

jasnell commented Mar 2, 2016

Copy link
Copy Markdown
Member

Added the lts watch label...

Knighton910 commented Mar 8, 2016

Copy link
Copy Markdown

Great stuff guys/gals

MylesBorins pushed a commit that referenced this pull request Mar 17, 2016
doc: fix relative links in net docs
a80e9a6 
They were not being processed as relative links.

PR-URL: #5358
Reviewed-By: Roman Reiss <me@silverwind.io>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
doc: fix relative links in net docs
901dbab 
They were not being processed as relative links.

PR-URL: #5358
Reviewed-By: Roman Reiss <me@silverwind.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL