Skip to content
This repository was archived by the owner on Aug 5, 2022. It is now read-only.

Update WebIDL and modify documentation to fit new "node.js-like" style, Part 2 #1893

Merged
merged 30 commits into from
Jul 13, 2018
Merged

Update WebIDL and modify documentation to fit new "node.js-like" style, Part 2 #1893

merged 30 commits into from
Jul 13, 2018

Conversation

t-harvey
Copy link

@t-harvey t-harvey commented Jul 3, 2018

Folks,
This is the actual pull request for updating the documentation to: 1) follow the style of node.js and 2) correct the WebIDL.

I tried to incorporate the advice/corrections from the original test pull request (#1800).

As with the test pull request, I've limited myself to fixing grammatical errors and the actual WebIDL, but the documentation could definitely do with a thorough going over for technical detail as well.

That said, I've run all of the updated WebIDL through my own parser, and it all builds correctly. I wrote an automated tester for this, but I'm largely a newbie, so I don't know where I can put it and how to get it to run as part of the automated testing; any advice, pointers, etc. would be very useful.

Thanks,
Tim

Timothy Harvey and others added 21 commits January 24, 2018 16:11
be syntactically correct and to follow the new style of buffer.md.

Added a new file that explains some of the differences between WebIDL
and Javascript.
Some changes to a few .md files to get feedback.
Modified format and fixed WebIDL
…#1881)

Thanks to Paul Sokolovsky for finding this solution.

Signed-off-by: Geoff Gustafson <[email protected]>
Created shim layer for building against the POSIX network stack on
Zephyr which enables the debugger to run on the frdm_k64f board with
TCP IPV4 on port 5001.

Signed-off-by: Jimmy Huang <[email protected]>
Signed-off-by: Timothy Harvey <[email protected]>
Signed-off-by: Timothy Harvey <[email protected]>
@t-harvey t-harvey closed this Jul 4, 2018
@t-harvey t-harvey reopened this Jul 4, 2018
@t-harvey t-harvey closed this Jul 4, 2018
@t-harvey t-harvey reopened this Jul 4, 2018
@jimmy-huang
Copy link
Contributor

jimmy-huang commented Jul 9, 2018

@t-harvey looks good to me, thank you for sending this patch and help us fixing up the web-idls, definilte a lot better than the previous one, we are okay with merging this patch as is, but I would like to know if there's a reason to use <p> for all these new lines? Is this like a editor-generated styles that you were using or this is the standard practice for webidls?

@t-harvey
Copy link
Author

t-harvey commented Jul 9, 2018

Jimmy,
In the "test" pull request for updating the docs, one of the reviewers wanted the WebIDL put into a separate file and just linked to -- but I didn't think this worked very well, so I decided I'd try using the markdown feature called "details", which, as you can see, lets you hide away details until you need them.
Unfortunately, the text inside "details" is interpreted as html rather than as markdown, so we have to use html formatting there -- and I soon learned that without using the <p>'s, all of the indentation goes away.
Thus, we're stuck with some ugly text that displays nicely.
If you've got a better method (b/c I'm not an html expert!), I'm all ears...but as I said, I've got a script that rips out the WebIDL for testing, and it was trivial to get it to ignore <p>, so it's easily worked around, and you and I are the only ones who'll ever see the awkwardness of the text... :-)

@grgustaf
Copy link
Contributor

grgustaf commented Jul 9, 2018

Yeah, I looked through it too and looks good. Since it's all in documentation there should be no problem with just merging it. :)

@t-harvey are you sure you still needed the <p> tags after you started using <pre>? I think that takes all your newlines literally. So I suspect you could remove all those. Also don't need the double blank lines in some places.

docs/ble.md Outdated
// require returns a BLE object
// var ble = require('ble');

[NoInterfaceObject]
<p><p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's where there are a bunch of double blanks; there are others around though.

Copy link
Author

Choose a reason for hiding this comment

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

Geoff,
Since both you and Jimmy noticed, this, I'll go back and rip out as many as possible... :-)

Copy link
Author

Choose a reason for hiding this comment

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

...also, I, too, thought the pre-construct inside of the details-construct would suffice for formatting, but I just went back and checked, and it doesn't do the trick; the only thing that worked was to wrap the pre-construct around the details-construct (instead of the other way around), but then the pulldown menu is in the wrong font, which looks ugly...

I can make this switch (i.e., put the pre-construct first), if you guys want, but as the html-newlines are invisible to everyone except us, I'm more inclined to leave it pretty but (mildly) cumbersome. :-)

I've uploaded one file to show what swapping the two constructs looks like -- once Travis has its way with the docs, compare/contrast the docs for i2c.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think the old way was better.

Copy link
Author

Choose a reason for hiding this comment

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

Great -- I'll clean up the git record (so expect a --force'd commit :-), remove as many newlines as I can aesthetically :-), and put in a final upload.

@grgustaf
Copy link
Contributor

LGTM +1

@grgustaf grgustaf merged commit 6d21e17 into intel:master Jul 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants