Standardizing h3/h4/h4 interface and dictionary markup. See Issue #2316 #2317

skratchdot · 2021-04-09T14:20:05Z

For more info, see Issue #2316

I've updated the markup for 37 interfaces and 29 dictionaries. For a list of links, please see the following gist:
https://gist.github.com/skratchdot/6f98d30ae8d7a3de01feb475c4b6df93#file-updated-idls-md

Out of the 37 interfaces in the spec, this PR standardizes the markup for the 36 interfaces that have
header tags (and show up in the left navigation). There is 1 interface in the spec that does not have
it's own header/left nav entry:

AudioParamMap

Here is the script I used to update the index.bs file:
https://gist.github.com/skratchdot/6f98d30ae8d7a3de01feb475c4b6df93#file-update-index-sh

#!/bin/bash

interfaces="AnalyserNode AudioBuffer AudioBufferSourceNode AudioContext AudioDestinationNode AudioListener AudioNode AudioParam AudioParamMap AudioProcessingEvent AudioScheduledSourceNode AudioWorklet AudioWorkletGlobalScope AudioWorkletNode AudioWorkletProcessor BaseAudioContext BiquadFilterNode ChannelMergerNode ChannelSplitterNode ConstantSourceNode ConvolverNode DelayNode DynamicsCompressorNode GainNode IIRFilterNode MediaElementAudioSourceNode MediaStreamAudioDestinationNode MediaStreamAudioSourceNode MediaStreamTrackAudioSourceNode OfflineAudioCompletionEvent OfflineAudioContext OscillatorNode PannerNode PeriodicWave ScriptProcessorNode StereoPannerNode WaveShaperNode"
dictionaries="AnalyserOptions AudioBufferOptions AudioBufferSourceOptions AudioContextOptions AudioNodeOptions AudioParamDescriptor AudioProcessingEventInit AudioTimestamp AudioWorkletNodeOptions BiquadFilterOptions ChannelMergerOptions ChannelSplitterOptions ConstantSourceOptions ConvolverOptions DelayOptions DynamicsCompressorOptions GainOptions IIRFilterOptions MediaElementAudioSourceOptions MediaStreamAudioSourceOptions MediaStreamTrackAudioSourceOptions OfflineAudioCompletionEventInit OfflineAudioContextOptions OscillatorOptions PannerOptions PeriodicWaveConstraints PeriodicWaveOptions StereoPannerOptions WaveShaperOptions"

for i in $interfaces; do
  lcase="$(tr [A-Z] [a-z] <<< "$i")"
  gsed -i -E "s/^<h3 .*[=\"]$i[\" \>].*$/<h3 interface lt=\"$lcase\" id=\"$i\">/" index.bs
  gsed -i -E "s/^<h4 .*[=\"]$i[\" \>].*$/<h4 interface lt=\"$lcase\" id=\"$i\">/" index.bs
done

for i in $dictionaries; do
  lcase="$(tr [A-Z] [a-z] <<< "$i")"
  gsed -i -E "s/^<h4 .*[=\"]$i[\" \>].*$/<h4 dictionary lt=\"$lcase\" id=\"$i\">/" index.bs
  gsed -i -E "s/^<h5 .*[=\"]$i[\" \>].*$/<h5 dictionary lt=\"$lcase\" id=\"$i\">/" index.bs
done

If we ever need to change the h3/h4/h4 markup again, we can probably re-use something similar.

💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Apr 13, 2021, 11:17 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.

🔗 Related URL

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>500 Internal Server Error</title>
</head><body>
<h1>Internal Server Error</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator at 
 [email protected] to inform them of the time this error occurred,
 and the actions you performed just before this error.</p>
<p>More information about this error may be available
in the server error log.</p>
</body></html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

w3cbot · 2021-04-09T14:21:36Z

dontcallmedom marked as non substantive for IPR from ash-nazg.

skratchdot · 2021-04-09T14:26:46Z

It appears a few links above are "broken", so I either need to remove them or "fix" them. I will try to annotate what's working and what's not (and compare to the current published doc).

skratchdot · 2021-04-09T15:05:04Z

It might make more sense semantically to have:

<h3 interface lt="NameOfInterface" id="nameofinterface">

I should test the behavior of that as well!

skratchdot · 2021-04-10T15:26:59Z

Okay- I found the issue from yesterday- I was not scripting out changes to h4 tags as well. I've updated the main PR description and this is ready to merge (from my perspective). Let me know if I should change anything else!

rtoy · 2021-04-12T14:53:26Z

I think there are a couple of things wrong here (but not with the actual changes to the index.bs.)

First, the merge request should be to the main branch, not to the gh-pages branch. Second, do not check in index.html.

Travis CI will take care of creating index.html automatically, and when it's merged, it will update everything needed to get the spec generated in the right place/branch.

This might also explain when the preview/diff didn't work.

skratchdot · 2021-04-12T15:45:55Z

I think there are a couple of things wrong here (but not with the actual changes to the index.bs.)

First, the merge request should be to the main branch, not to the gh-pages branch. Second, do not check in index.html.

Travis CI will take care of creating index.html automatically, and when it's merged, it will update everything needed to get the spec generated in the right place/branch.

This might also explain when the preview/diff didn't work.

Interesting. I didn't even have a main branch locally (I think due to how long ago I forked this repo). Anyways, I think I've fixed things this time. I've rebased and changed the base branch. Hopefully the preview site works this time! 🤞

skratchdot · 2021-04-12T15:55:07Z

Wondering if the pr build/preview is failing due to the source org being my github username?
I'm seeing a travis job failing:
https://travis-ci.com/github/skratchdot/web-audio-api/builds/222837896
🤷

rtoy · 2021-04-12T20:43:01Z

I've seen that rvm failure before. Not sure how I fixed it or if I just ignored it because it was on my fork, and not the official spec repo.

rtoy · 2021-04-12T20:51:45Z

I can't currently test the changes since preview is not working, but I have a question. If you navigate to PeriodicWave heading, the first paragraph has a link to PeridocWave. If you click it, were does it go? Currently for me it goes to the section. Does it now go to the IDL? The links to AudioContext seem to go to the IDL section. This makes more sense to me since bikeshed already has special syntax to reference sections.

skratchdot · 2021-04-12T22:31:36Z

I can't currently test the changes since preview is not working, but I have a question. If you navigate to PeriodicWave heading, the first paragraph has a link to PeridocWave. If you click it, were does it go? Currently for me it goes to the section. Does it now go to the IDL? The links to AudioContext seem to go to the IDL section. This makes more sense to me since bikeshed already has special syntax to reference sections.

In the "API Overview" section- all the links take me to the IDL definitions. Only the links in the "left nav" take me to the section heading. So if this PR is merged, any {{PeriodicWave}} will link to the IDL definition. We can make those link to the header, but if we do that- then there will be no links to the IDL (as far as I can tell). I am also noticing some inconsistant html in the "API Overview" section:

web-audio-api/index.bs

Line 513 in e29a1cd

* An <a class="dfnref" href="#AudioContext">AudioContext</a>

It seems like that should read:
An {{AudioContext}} to be consistent with the rest of that section.

I don't really have a preference to where things link, but it seems like whichever "solution" we go with, all the h3/h4 markup for the interfaces should be consistent.

skratchdot · 2021-04-12T22:46:59Z

I need to read up more on bikeshed. If I'm reading this correctly:
https://tabatkins.github.io/bikeshed/#autolinking

I think {{PeriodicWave}} should link to the IDL definition and [[PeriodicWave]] will link to the header? I need to test this.

Also, reading the data model contract:
https://tabatkins.github.io/bikeshed/#dfn-contract

I don't think I'm actually doing part 4 data-dfn-type MUST be provided, and set one of the accepted values. But maybe the interface attribute is shorthand for data-dfn-type="interface"? 🤷

rtoy · 2021-04-13T15:34:16Z

I think we're ok here. I don't think there's much use of [[foo]] for section references (but we do use a lot of [[foo]] to mean an internal slot). So all links like {{foo}} should go to the definition of foo, which is usually the IDL defining it or the function/attribute description.

I think none of the headings are actually defining any term, so what you have for the headings seems fine.

rtoy · 2021-04-13T19:31:37Z

Grabbed your branch and built it locally. I get some new bikeshed errors:

+LINE -1: Couldn't find target document section audioworklet:
+[[#audioworklet|direct script processing and synthesis]]
+LINE ~295: Couldn't find target document section audioworklet:
+[[#audioworklet|directly using scripts]]
+LINE ~325: Couldn't find target document section analysernode:
+[[#analysernode|real-time time-domain and frequency-domain analysis / music visualizer support]]
  ✔  Successfully generated, but fatal errors were suppressed

I think with the new scheme, the links here should be using [[#CamelCase]] to get to the section.

I didn't check all the links, but the ones I did test go to the IDL section as we wanted.

skratchdot · 2021-04-13T21:40:37Z

@rtoy - Good call on the bikeshed errors. At some point I started ignoring those and just confirming the links.

I fixed the errors you mentioned in 245578f

But I'm now seeing:

LINE 2022: Can't find the 'contextOptions' argument of method 'OfflineAudioContext/constructor(numberOfChannels, length, sampleRate)' in the argumentdef block.
LINE 3129: Can't find the 'destinationNode' argument of method 'AudioNode/connect(destinationParam, output)' in the argumentdef block.
LINE 3129: Can't find the 'input' argument of method 'AudioNode/connect(destinationParam, output)' in the argumentdef block.
LINE 3230: Can't find the 'destinationNode' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
LINE 3243: Can't find the 'destinationNode' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
LINE 3258: Can't find the 'destinationNode' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
LINE 3258: Can't find the 'input' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
 ✔  Successfully generated, but fatal errors were suppressed

I'm not sure if those should be fixed in this PR or what.

When I compile the main branch, I see a similar set of errors:

LINE 2022: Can't find the 'contextOptions' argument of method 'OfflineAudioContext/constructor(numberOfChannels, length, sampleRate)' in the argumentdef block.
LINE 3129: Can't find the 'destinationNode' argument of method 'AudioNode/connect(destinationParam, output)' in the argumentdef block.
LINE 3129: Can't find the 'input' argument of method 'AudioNode/connect(destinationParam, output)' in the argumentdef block.
LINE 3230: Can't find the 'destinationNode' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
LINE 3243: Can't find the 'destinationNode' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
LINE 3258: Can't find the 'destinationNode' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
LINE 3258: Can't find the 'input' argument of method 'AudioNode/disconnect(destinationParam, output)' in the argumentdef block.
 ✔  Successfully generated, but fatal errors were suppressed

rtoy · 2021-04-13T21:44:36Z

You should use compile.sh to run bikeshed. This compares the expected errors with the actual errors. If there are differences, the output will list them. If compile.sh says there are no differences, we're all set.

Travis CI passes, so I think everything is fine now.

skratchdot · 2021-04-13T21:45:44Z

Yeah- I see travis passing now:
https://travis-ci.com/github/skratchdot/web-audio-api/builds/222988145

But seeing:

Skipping a deployment with the pages provider because this branch is not permitted: issue-2316

(I think b/c the branch is in my repo)

rtoy · 2021-04-13T21:51:58Z

I think that's ok, because only changes to the main branch should be deployed. Don't know why the preview is failing though.

While randomly clicking on headers, I noticed that AudioBufferSourceOptions section doesn't link to the IDL. The other dictionaries also end up at the section heading, not he IDL. These might need to be fixed too.

skratchdot · 2021-04-13T22:07:19Z

I think that's ok, because only changes to the main branch should be deployed. Don't know why the preview is failing though.

While randomly clicking on headers, I noticed that AudioBufferSourceOptions section doesn't link to the IDL. The other dictionaries also end up at the section heading, not he IDL. These might need to be fixed too.

Good call. I can update the dictionaries as well.

skratchdot · 2021-04-13T22:19:54Z

I updated the dictionary markup in 1ebd936

I need to update the description with links to the 29 dictionaries. I'll start testing that all 58 (29*2) of the links in that commit "work"

skratchdot · 2021-04-13T22:30:23Z

All 58 of the dictionary links seem to work with 1 exception:

IIRFilterOptions

For some reason, when you click IIRFilterOptions in the header section- it doesn't take you to the IDL- and the "popup" opens to the right of the page. Kind of weird.

Also, all the IDL links have dictdef- prepended to them

rtoy · 2021-04-13T22:30:50Z

For reference, the original webaudio bikehsed issue is #2185, which is now being tracked in speced/bikeshed#1740.

rtoy · 2021-04-13T22:36:04Z

Ah, for the IIRFilterOptions link, I think you need to say:

<h4 dictionary ...>
{{IIRFitlerOptions}}</h4>

to get the section heading to link to the IIRFilterOptions IDL.

skratchdot · 2021-04-13T22:36:38Z

Ah, for the IIRFilterOptions link, I think you need to say:
<h4 dictionary ...>
{{IIRFitlerOptions}}</h4>
to get the section heading to link to the IIRFilterOptions IDL.

Yeah- I was just now fixing that :) Should be fixed in 9910392

rtoy · 2021-04-13T22:45:50Z

Assuming the links to your repo aren't really stable, I would not include them in the description. You can just list all the nodes/headers/dictionaries that you updated.

(I think the description is what gets committed as the merge log. I forget if that's exactly true. I think you also get an option to edit the message before merging.)

skratchdot · 2021-04-13T22:53:03Z

Assuming the links to your repo aren't really stable, I would not include them in the description. You can just list all the nodes/headers/dictionaries that you updated.

(I think the description is what gets committed as the merge log. I forget if that's exactly true. I think you also get an option to edit the message before merging.)

Good point. I'll just put them in a gist (with versions that point to my repo- and the real repo), that way we can confirm before and after merging the PR.

rtoy · 2021-04-13T23:02:16Z

Excellent. Since a gist is stable, more or less, a link to the gist is fine.

Also, if you used a simple sed/awk/perl script to do this, pasting the script into the description is helpful.

I looked over everything and it looks fine. I did some random testing and everything is fine too.

The hard part is if there were some stray links that now no longer work. But I think we can fix those as we discover them.

rtoy

Looks good. Thanks for doing this! It's a lot of work, but very well worth it to make the spec nice and clean and usable.

skratchdot · 2021-04-13T23:09:29Z

@rtoy - I updated the description. I have a gist with a few files in it. The script I used to write the links is there:
https://gist.github.com/skratchdot/6f98d30ae8d7a3de01feb475c4b6df93#file-write-links-sh

I just slightly modified it to generate the following files:

The script I made to modify the index.bs file is here:
https://github.com/skratchdot/web-audio-api/blob/gh-pages/index-with.sh

I will clean that up and paste in the description.

rtoy · 2021-04-13T23:24:10Z

Everything looks fine. When you are satisfied with everything, let me know, and we can merge this in.

skratchdot · 2021-04-13T23:28:30Z

Everything looks fine. When you are satisfied with everything, let me know, and we can merge this in.

I'm done making changes (unless there's anything else we find) 😃. I hope there are no unintended consequences 🤞 I did manually click all the links in:
https://gist.github.com/skratchdot/6f98d30ae8d7a3de01feb475c4b6df93#file-links-skratchdot-md
and things looked good, but it's definitely hard to confirm "everything still works"!

Hopefully all goes smoothly after the merge :)

rtoy · 2021-04-14T15:22:20Z

Looks good. The new version is live now and doing some random checking works as expected. Thanks!

skratchdot force-pushed the issue-2316 branch 2 times, most recently from c6b4f51 to de1814f Compare April 9, 2021 14:57

skratchdot changed the title ~~Standardizing h3 interface markup. See Issue #2316~~ Standardizing h3/h4 interface markup. See Issue #2316 Apr 10, 2021

Standardizing h3/h4 interface markup. See Issue WebAudio#2316

5a1ec8b

skratchdot force-pushed the issue-2316 branch from ec5e851 to 5a1ec8b Compare April 12, 2021 15:43

skratchdot changed the base branch from gh-pages to main April 12, 2021 15:43

rtoy added this to the Web Audio V1 milestone Apr 13, 2021

Fixing bikeshed errors

245578f

rtoy self-requested a review April 13, 2021 22:16

Standardizing h4/h5 dictionary markup.

1ebd936

Fix IIRFilterOptions header

9910392

skratchdot changed the title ~~Standardizing h3/h4 interface markup. See Issue #2316~~ Standardizing h3/h4/h4 interface and dictionary markup. See Issue #2316 Apr 13, 2021

rtoy approved these changes Apr 13, 2021

View reviewed changes

rtoy merged commit 0e5ff24 into WebAudio:main Apr 14, 2021

Standardizing h3/h4/h4 interface and dictionary markup. See Issue #2316 #2317

Standardizing h3/h4/h4 interface and dictionary markup. See Issue #2316 #2317

Uh oh!

Conversation

skratchdot commented Apr 9, 2021 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💥 Error: 500 Internal Server Error 💥

Uh oh!

w3cbot commented Apr 9, 2021

Uh oh!

skratchdot commented Apr 9, 2021

Uh oh!

skratchdot commented Apr 9, 2021

Uh oh!

skratchdot commented Apr 10, 2021

Uh oh!

rtoy commented Apr 12, 2021

Uh oh!

skratchdot commented Apr 12, 2021

Uh oh!

skratchdot commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rtoy commented Apr 12, 2021

Uh oh!

rtoy commented Apr 12, 2021

Uh oh!

skratchdot commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skratchdot commented Apr 12, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

rtoy left a comment

Choose a reason for hiding this comment

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

rtoy commented Apr 13, 2021

Uh oh!

skratchdot commented Apr 13, 2021

Uh oh!

rtoy commented Apr 14, 2021

Uh oh!

Uh oh!

skratchdot commented Apr 9, 2021 •

edited by pr-preview bot

Loading

skratchdot commented Apr 12, 2021 •

edited

Loading

skratchdot commented Apr 12, 2021 •

edited

Loading