Closed Bug 966720 Opened 6 years ago Closed 6 years ago

[jsdbg2] Debugger specification should be in-tree

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jimb, Assigned: jimb)

Details

Attachments

(2 files, 6 obsolete files)

The specification for the Debugger API should be checked into the tree, alongside the SpiderMonkey sources. This way, we can:

- distinguish implemented from not-yet-implemented features
- require doc updates with patches
- track historical versions

all in the obvious way. The github repository that holds the docs now will be shut down.

I would like to write the docs in Markdown, because that will be most legible in patches. Tools exist to convert Markdown to Mediawiki markup, so we can keep the wiki up to date.
Okay, I've gotten all the docs converted to Markdown now, and made legible through The Glory of Emacs. I'm still not sure how to get internal link anchors placed correctly, but surely that can be addressed somehow. I'd hate to lose all this work, so I'm attaching the work-in-progress patch.
Assignee: nobody → jimb
Status: NEW → ASSIGNED
This version includes a first cut at some scripts to automatically format the pages from markdown into HTML, and some meta-documentation for people working on the documentation.

Next: scripts to automatically publish the docs to MDN.
Attachment #8381924 - Attachment is obsolete: true
This patch only adds the docs themselves, not the supporting scripts. This should be easy to review.
Attachment #8396078 - Attachment is obsolete: true
Attachment #8396080 - Flags: review?(jorendorff)
Here are the support scripts, split out into their own patch. This is probably more interesting to review.
Attachment #8396081 - Flags: feedback?(jorendorff)
In the scripts patch, ignore post.sh; that part isn't done yet.
Comment on attachment 8396080 [details] [diff] [review]
Check Debugger docs into the SpiderMonkey tree.

Review of attachment 8396080 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't look at the PNGs or read much of the text, just spot-checked for markdown sanity.

r=me on the idea; you'll certainly have better luck getting people to contribute to markdown docs than the old github repo, and it makes sense to keep docs and source together.

::: js/src/doc/Debugger/Conventions.md
@@ +1,3 @@
> +# General Conventions
> +
> +This page describes general conventions used in the [`Debugger`][debugger] API,

I thought this syntax: `[text][id]` meant `<a href="${url}">text</a>` and the url had to be defined later in the document, like so:

    [id]: http://example.com/  "Optional Title Here"

But none of these ids seem to be defined -- it must be filled in by a script later, or something...
Attachment #8396080 - Flags: review?(jorendorff) → review+
Comment on attachment 8396081 [details] [diff] [review]
Scripts to automatically format and publish Debugger docs from js/src/docs to MDN.

Review of attachment 8396081 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't read too closely, but close enough for an f+.

Editing and seeing changes is not as simple as doing so at github. It's the right thing even so. I'm slightly concerned that `cabal install pandoc` didn't just work for me though. Updating Cabal now...

::: js/src/doc/lib/post.sh
@@ +5,5 @@
> +#
> +#   sh post.sh KEYID SECRET
> +#
> +# where KEYID and SECRET are your developer.mozilla.org API keys, generated here:
> +#   https://developer.mozilla.org/en-US/keys/

Splendid comment! This answered my question.
Attachment #8396081 - Flags: feedback?(jorendorff) → feedback+
This version can post to MDN, detecting when the site has actually changed. Some minor issues left:

- Should the formatted-for-inspection pages be left in js/src/doc, or in the build tree? It seems like we shouldn't be putting stuff in the source tree.

- Shouldn't the publish script avoid overwriting changes made directly to the wiki? Granted, they'll be retained in the page history, but it's not the smoothest interaction.

- The watermarks are simply stuck on the end of the text, as I was unable to find a way to hide them in the HTML that Kuma didn't strip out. I'll talk with the MDN folks and see how best to save this information.
Attachment #8396081 - Attachment is obsolete: true
Oh, and curl --silent still prints HTTP response codes.
D'oh, forgot to add the interesting scripts to the patch.
Attachment #8398902 - Attachment is obsolete: true
> [...] I'm slightly concerned that `cabal install pandoc`
> didn't just work for me though. Updating Cabal now...

Fixed six months ago; there just hasn't been a pandoc release since then:
  https://github.com/jgm/pandoc/issues/986

`cabal install happy alex pandoc` is the workaround, I guess? Or of course you can just download a binary build for your platform.

-j
Is cabal installed by default? What are the full pandoc installation instructions?
Flags: needinfo?(jorendorff)
Here's a revision with the uploading and subdir configuration working the way it should. Much simpler when it's done right.
Attachment #8398904 - Attachment is obsolete: true
Attachment #8400318 - Flags: review?(jorendorff)
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f56669bdfdf
Flags: in-testsuite-
Whiteboard: [leave open]
Comment on attachment 8400318 [details] [diff] [review]
v4: Scripts to automatically format and publish Debugger docs from js/src/docs to MDN.

Un-requesting review; there are some improvements that need to be made...
Attachment #8400318 - Flags: review?(jorendorff)
cabal is the GHC package manager. It's not installed by default, and we shouldn't recommend that people use it; I used it because I happen to have it.

Installing pandoc: http://www.johnmacfarlane.net/pandoc/installing.html
Flags: needinfo?(jorendorff)
(In reply to Jason Orendorff [:jorendorff] from comment #17)
> Installing pandoc: http://www.johnmacfarlane.net/pandoc/installing.html

Linking to that is obviously the correct thing. Thanks.
Okay, this generates correct links for both on-disk and on-wiki reading, and tells people where the official source file for each page is located. Ready for review.
Attachment #8400318 - Attachment is obsolete: true
Attachment #8404301 - Flags: review?(jorendorff)
Comment on attachment 8404301 [details] [diff] [review]
v5: Scripts to automatically format and publish Debugger docs from js/src/docs to MDN.

Review of attachment 8404301 [details] [diff] [review]:
-----------------------------------------------------------------

Well ... obviously having this checked in is better than not having it checked in, so let's get it in.

The duplication is unfortunate.

::: js/src/doc/README.md
@@ +155,5 @@
> +Indeed it should. The config.sh files should just become moz.build files.
> +And it should somehow be reconciled / contrasted / unified / integrated
> +with 'mach build-docs', which seems to format in-tree docs of a different
> +kind, and use a different markup language (ReStructuredText) and a
> +different formatter (Sphynx).

It's spelled "Sphinx", fwiw.

Style nit: I would kill the second sentence in this paragraph. Also I would ditch the slashes and pick a verb. "Unified" is nice.

::: js/src/doc/lib/extract-watermark.sh
@@ +7,5 @@
> +# retrieved from MDN, to see if anything needs to be updated.
> +#
> +# Usage:
> +#
> +#     extract-watermark.sh

Typical usage involves curl, though, right?

::: js/src/doc/lib/make-watermark.sh
@@ +15,5 @@
> +echo '<dl>'
> +echo '    <dt>Generated from file:<dt>'
> +echo "    <dd>$2</dd>"
> +echo '    <dt>Watermark:</dt>'
> +echo "    <dd id='watermark'>sha256:$(cat "$1" | sha256sum | sed 's/ .*$//')</dd>"

sha256sum is a linux-ism. Macs have this equivalent:
    shasum -a 256

@@ +18,5 @@
> +echo '    <dt>Watermark:</dt>'
> +echo "    <dd id='watermark'>sha256:$(cat "$1" | sha256sum | sed 's/ .*$//')</dd>"
> +
> +# If we have Mercurial changeset ID, include it.
> +if [ "${JS_DOC_HG_IDENTIFY:+set}" = set ]; then

I had to look this one up. Phew.
Attachment #8404301 - Flags: review?(jorendorff) → review+
Thanks for the review.

(In reply to Jason Orendorff [:jorendorff] from comment #20)
> It's spelled "Sphinx", fwiw.
> 
> Style nit: I would kill the second sentence in this paragraph. Also I would
> ditch the slashes and pick a verb. "Unified" is nice.

Fixed.

> ::: js/src/doc/lib/extract-watermark.sh
> @@ +7,5 @@
> > +# retrieved from MDN, to see if anything needs to be updated.
> > +#
> > +# Usage:
> > +#
> > +#     extract-watermark.sh
> 
> Typical usage involves curl, though, right?

Yes. I've added an example to the docs.

> sha256sum is a linux-ism. Macs have this equivalent:
>     shasum -a 256

That works on Linux, too, so I've changed the code to use that.
Whiteboard: [leave open]
https://hg.mozilla.org/mozilla-central/rev/1ae3c542e431
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.