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: parse documentation metadata #3867

Closed
wants to merge 1 commit into from

Conversation

tflanagan
Copy link
Contributor

This commit introduces the Documentation YAML metadata concept to the
documentation.

  • Parses added or Added for HTML
  • Parses all data for JSON

This is in prelude to #3713.

Markup would look like this:

## assert(value[, message]), assert.ok(value[, message])
<!-- YAML
added: v0.10.0
-->

@jasnell
Copy link
Member

jasnell commented Nov 16, 2015

LGTM

@mscdex mscdex added the tools Issues and PRs related to the tools directory. label Nov 16, 2015
@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

@nodejs/documentation

@bengl
Copy link
Member

bengl commented Dec 16, 2015

Could this possibly include examples in tools/doc/README.md?

@tflanagan
Copy link
Contributor Author

@bengl, I've added examples

/poke

@bengl
Copy link
Member

bengl commented Jan 22, 2016

LGTM

@Qard
Copy link
Member

Qard commented Jan 22, 2016

Hey, so I'm working on replacing our doctool with remark, hopefully sooner than later. Part of that work is going to be to parsing YAML metadata after headers, which could be used for this sort of thing.

I just wanted to share that bit of info because this data seems like it'd fit well into YAML metadata, so it'd be good to keep that in mind.

@tflanagan
Copy link
Contributor Author

@Qard, looks like a format is still being debated?

@bengl
Copy link
Member

bengl commented Jan 22, 2016

@tflanagan I think it's better to merge what you have here so folks can get started annotating the docs. We can change to the new YAML format later, once it's been solidified.

@chrisdickinson
Copy link
Contributor

It just occurred to me to ask this: does adding <sup> info to an existing header change the generated anchor?

@tflanagan
Copy link
Contributor Author

No,

var id = getId(filename + '_' + removeHeadingSup(tok.text.trim()));

@jasnell
Copy link
Member

jasnell commented Jan 23, 2016

+1 to getting this landed and incrementally improving from there.

@chrisdickinson
Copy link
Contributor

Some background: the direction we are planning on heading is to add per-section YAML data in an HTML comment below the section header — that way we can store all sorts of metadata about the section, including "since version" info. Concretely, this is what that would look like. In any case, we'll need the "since version" information annotated, but we might introduce a little extra churn if we go the <sup> route.

So, I'm +1 on merging this as-is, with the knowledge that we may be revisiting docs that include <sup> with a subsequent PR to move that information to a YAML section, or +1 on starting out in the HTML comment path. Sorry for the back-and-forth — we're working on a Docs roadmap to make sure that this sort of discussion doesn't take folks by surprise in the future.

@jasnell
Copy link
Member

jasnell commented Jan 23, 2016

sounds like a solid approach to me. btw, I've been wanting to join the docs wg discussions more but have been buried the past two weeks. I'm looking forward to seeing what more y'all have come up with so far.

@Qard
Copy link
Member

Qard commented Jan 23, 2016

I'm also 👍 on this, just wanted to share some details on the doctool we're working on and how that will impact the content of this PR in the future.

If you want to see what we're working on for the doctool, there's a tracking issue with a list of the parts we need to put together. 😸

@tflanagan
Copy link
Contributor Author

Awesome, I'd love to get this landed too.

But this requires some CSS to go along with it. I wasn't sure if that was something I should introduce or something that the Doc team would take care of, but as of right now, this PR would make the Docs look a bit off...

@jasnell
Copy link
Member

jasnell commented Jan 25, 2016

That should likely be included in this PR then if possible.
On Jan 25, 2016 6:25 AM, "Tristian Flanagan" notifications@github.com
wrote:

Awesome, I'd love to get this landed too.

But this requires some CSS to go along with it. I wasn't sure if that was
something I should introduce or something that the Doc team would take care
of, but as of right now, this PR would make the Docs look a bit off...


Reply to this email directly or view it on GitHub
#3867 (comment).

@tflanagan
Copy link
Contributor Author

The HTML API is rather constraining in terms of width... Which makes putting it next to either the hashtag static link or the method/prop/class/etc name a little hard.

We either need to expand the width of the #apicontent div or move this since ver tag somewhere else

@Qard
Copy link
Member

Qard commented Jan 25, 2016

I was thinking for the YAML formatted stuff in the future doctool, it could put a block of stuff below the header. You could maybe try that here to start us in that direction.

@nodejs/documentation Does that sound like a reasonable plan?

@tflanagan
Copy link
Contributor Author

@Qard, That's a good idea. I'll submit a solution by EOD

@tflanagan tflanagan force-pushed the doc-parse-since-ver branch 2 times, most recently from ffcb651 to 19e0bc8 Compare January 25, 2016 21:56
@tflanagan
Copy link
Contributor Author

@jasnell, @Qard, so here's the first draft, pretty basic, but gets the concept out there.

## assert(value[, message]), assert.ok(value[, message])
<!-- YAML
stability: 0
added: v0.10.0
deprecated: v4.0.0
-->

This implementation allows for and already supports any other metadata (stability, deprecated) you wish to add.

assert_meta

I've also, done a little touch up on generate.js.

json.js adds a meta property:

{
  "textRaw": "assert(value[, message]), assert.ok(value[, message])",
  "type": "method",
  "name": "ok",
  "meta": {
    "stability": 0,
    "added": "v0.10.0",
    "deprecated": "v4.0.0"
  },
  "desc": "<p>Tests if <code>value</code> is truthy. It is equivalent to\n<code>assert.equal(!!value, true, message)</code>.\n\n</p>\n<p>If <code>value</code> is not truthy, an <code>AssertionError</code> is thrown with a <code>message</code>\nproperty set equal to the value of the <code>message</code> parameter. If the <code>message</code>\nparameter is <code>undefined</code>, a default error message is assigned.\n\n</p>\n<pre><code class=\"js\">const assert = require(&#39;assert&#39;);\n\nassert(true);  // OK\nassert(1);     // OK\nassert(false);\n  // throws &quot;AssertionError: false == true&quot;\nassert(0);\n  // throws &quot;AssertionError: 0 == true&quot;\nassert(false, &#39;it\\&#39;s false&#39;);\n  // throws &quot;AssertionError: it&#39;s false&quot;\n\nassert.ok(true);  // OK\nassert.ok(1);     // OK\nassert.ok(false);\n  // throws &quot;AssertionError: false == true&quot;\nassert.ok(0);\n  // throws &quot;AssertionError: 0 == true&quot;\nassert.ok(false, &#39;it\\&#39;s false&#39;);\n  // throws &quot;AssertionError: it&#39;s false&quot;</code></pre>\n",
  "signatures": [
    {
      "params": [
        {
          "name": "value"
        },
        {
          "name": "message])"
        },
        {
          "name": "assert.ok(value"
        },
        {
          "name": "message",
          "optional": true
        }
      ]
    }
  ]
}

Here is the CSS:

.api_metadata {
  font-size: .75em;
  margin-bottom: 1em;
}

.api_metadata span {
  margin-right: 1em;
}

.api_metadata span:last-child {
  margin-right: 0px;
}

@tflanagan tflanagan force-pushed the doc-parse-since-ver branch 3 times, most recently from ae6c941 to 6d1a70f Compare January 25, 2016 22:10
@tflanagan
Copy link
Contributor Author

That's fine, in terms of adding the actual data to the docs, I imagine something like one commit per doc file is what is desired?

Edit: I would do that in a different PR

@jasnell
Copy link
Member

jasnell commented Jan 27, 2016

Yeah, I'd think so, anything else becomes too difficult to rebase if things change while the PR is being reviewed.

@tflanagan tflanagan mentioned this pull request Jan 28, 2016
5 tasks
@tflanagan tflanagan changed the title tools: parse documentation heading "since version" tools: parse documentation metadata Feb 6, 2016
@tflanagan
Copy link
Contributor Author

/poke

@jasnell
Copy link
Member

jasnell commented Feb 19, 2016

/cc @nodejs/documentation

@Knighton910
Copy link

generally LGTM

@addaleax
Copy link
Member

@tflanagan Could you rebase this again?

@addaleax
Copy link
Member

ping @tflanagan again – still interested in this? if not, would you mind if someone else took this PR up again?

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Apr 30, 2016
@jasnell
Copy link
Member

jasnell commented Apr 30, 2016

@addaleax ... at this point I'd say if you'd like to pick this up go for it. It's been stalled for a while.

@addaleax
Copy link
Member

addaleax commented May 1, 2016

Continuing this in #6495.

addaleax pushed a commit that referenced this pull request May 4, 2016
This commit introduces the Documentation YAML metadata concept to the
documentation.

- Parses added or Added for HTML
- Parses all data for JSON

Ref: #3867
Ref: #3713
Ref: #6470
PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@addaleax
Copy link
Member

addaleax commented May 4, 2016

Pleased to announce that, after almost half a year, this has landed with some minor tweaking in 015f4fd! 🎉

@addaleax addaleax closed this May 4, 2016
evanlucas pushed a commit that referenced this pull request May 17, 2016
This commit introduces the Documentation YAML metadata concept to the
documentation.

- Parses added or Added for HTML
- Parses all data for JSON

Ref: #3867
Ref: #3713
Ref: #6470
PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
addaleax pushed a commit to addaleax/node that referenced this pull request Jul 12, 2016
This commit introduces the Documentation YAML metadata concept to the
documentation.

- Parses added or Added for HTML
- Parses all data for JSON

Ref: nodejs#3867
Ref: nodejs#3713
Ref: nodejs#6470
PR-URL: nodejs#6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
This commit introduces the Documentation YAML metadata concept to the
documentation.

- Parses added or Added for HTML
- Parses all data for JSON

Ref: #3867
Ref: #3713
Ref: #6470
PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
This commit introduces the Documentation YAML metadata concept to the
documentation.

- Parses added or Added for HTML
- Parses all data for JSON

Ref: #3867
Ref: #3713
Ref: #6470
PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
This commit introduces the Documentation YAML metadata concept to the
documentation.

- Parses added or Added for HTML
- Parses all data for JSON

Ref: #3867
Ref: #3713
Ref: #6470
PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stalled Issues and PRs that are stalled. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants