doc: fix JSON generation for aliased methods

[TimothyGu]

For example assert/assert.ok currently has the following signature:

    "signatures": [
      {
        "params": [
          {
            "name": "value"
          },
          {
            "name": "message])"
          },
          {
            "name": "assert.ok(value"
          },
          {
            "name": "message",
            "optional": true
          }
        ]
      }
    ]

While the heading reads

assert(value[, message]), assert.ok(value[, message])

Split them into two sections to make it working.

[evanlucas]

/cc @nodejs/documentation

[eljefedelrodeodeljefe]

Wouldn't it be better to fix it during JSON generation? Because the markdown renders fine and a change there doesn't seem to be something we should wish for, imo. What's your use case?

[tflanagan]

There is precedence for this type of change:

https://nodejs.org/api/https.html#https_server_listen_handle_callback

Just a friendly reminder that the whole doc generating process is pretty fragile, so I would +1 this change, as it already exists in the HTTPS docs.

[eljefedelrodeodeljefe]

Okay. Could we have a See assert.ok() for details. then? Because the difference is the that srv.listen() is one heading level deeper.

[TimothyGu]

Hi,

https://nodejs.org/api/buffer.html#buffer_buf_readdoublebe_offset_noassert

is not deeper and it uses the same style.

[TimothyGu]

Or actually it is. But I still don't see how that affects the argument.

[eljefedelrodeodeljefe]

It's bout consistency and readability. Also prettiness and semantics, since a heading without content looks like a bug to me. Even if there were precedence.

[tflanagan]

@eljefedelrodeodeljefe, you will find this style all over the docs. It is not a bug. It is by design.

The only argument against this is resorting the documentation which would cause duplicate documentation because we will have to move assert.ok down in the list.

As I have done this before, I am sure it is fine now.

[eljefedelrodeodeljefe]

Then make the case. -0

[tflanagan]

@eljefedelrodeodeljefe I would argue the case is FOR consistency and readability.

tbh, this was a mistake on my part in missing this when I reordered all the docs alphabetically, this should've already happened.

[TimothyGu]

@tflanagan that's a good point. How about I move ok to where it should be and link assert() to assert.ok()? This way it reduces redundancy and keeps the alphabetical order.

On the doc toolchain being fragile, I totally agree.

@eljefedelrodeodeljefe there are code in the toolchain to specifically handle this sort of headings: https://github.com/nodejs/node/blob/master/tools/doc/json.js#L38-L39. So I'd argue that the headings are like this by design.

[TimothyGu]

I have just splitted the section into two, so that the alphabetical order is preserved.

[chrisdickinson]

This LGTM. Making the headings more regular is great (even if it is to fix the JSON generator, which is pretty neglected!)

[thefourtheye]

All the other references in this document use the format assert.ok.

[TimothyGu]

This isn't true. See e.g. https://github.com/nodejs/node/blob/master/doc/api/assert.markdown#assertdoesnotthrowblock-error-message

[thefourtheye]

All I am saying is that, it should be consistent.

[eljefedelrodeodeljefe]

LGTM now.

[Trott]

Landed in 27c8b73

[jasnell]

lts-watch label applied

[MylesBorins]

This PR will have to wait for a backlog of doc fixes to land before we can land it in LTS

[Knighton910]

👍