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

Add support for the <wpt> element #90

Merged
merged 7 commits into from
Oct 21, 2018
Merged

Conversation

sideshowbarker
Copy link
Member

@sideshowbarker sideshowbarker commented Aug 26, 2018

This change adds support for generating output from the <wpt> element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116.

Specifically, this change causes lists of tests in <wpt> elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, https://wpt.fyi and https://web-platform-tests.live URLs for the listed tests.

The change by design intentionally doesn’t provide the following <wpt>-related features:

Fixes #87

@sideshowbarker sideshowbarker force-pushed the sideshowbarker/wpt-element branch 4 times, most recently from 645ff87 to 05e77f9 Compare August 26, 2018 11:47
src/wattsi.pas Outdated
WPTOutput.Add('<li class=wpt-test>');
WPTOutput.Add('<a href="https://wpt.fyi/results'
+ WPTPath + '">' + WPTFilename + '</a>');
WPTOutput.Add('<a href="http://web-platform-tests.live'
Copy link
Member

Choose a reason for hiding this comment

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

src/wattsi.pas Outdated
@@ -2015,6 +2051,41 @@ procedure Save(const Document: TDocument; const FileName: AnsiString; const InSp
if (Current is TElement) then
begin
Element := TElement(Current);
// TODO: Move the styles below to https://resources.whatwg.org/spec.css or
Copy link
Member

Choose a reason for hiding this comment

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

Address this

This change adds support for generating output from the `<wpt>` element,
as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and
discussed at speced/bikeshed#1116.

Specifically, this change causes lists of tests in `<wpt>` elements from
the source to generate TESTS sections in the spec output, with links to
corresponding https://github.com/web-platform-tests/wpt, https://wpt.fyi
and https://web-platform-tests.live URLs for the listed tests.

The change by design intentionally doesn’t provide the following
`<wpt>`-related features:

  - Doesn’t verify that each test listed in a `<wpt>` element
    actually exists in https://github.com/web-platform-tests/wpt/

  - Doesn’t verify that every single test file that exists in the
    https://github.com/web-platform-tests/wpt/tree/master/html tree is
    listed in a `<wpt>` element somewhere in the spec source.

Fixes #87
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/wpt-element branch from 014ab4d to 1649eff Compare August 30, 2018 15:41
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/wpt-element branch from 1649eff to cb86e15 Compare August 30, 2018 16:01
Copy link
Member

@zcorpan zcorpan left a comment

Choose a reason for hiding this comment

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

LGTM except for the TODO

@sideshowbarker
Copy link
Member Author

LGTM except for the TODO

OK, da2eea9 removes the embedded styles, and whatwg/whatwg.org#230 moves them to https://resources.whatwg.org/standard.css

@domenic
Copy link
Member

domenic commented Oct 20, 2018

The outputted HTML here differs from what I'm seeing in e.g. the bottom of https://quirks.spec.whatwg.org/#the-hashless-hex-color-quirk , which I assume is the HTML produced by Bikeshed. Among other things, this means we can't merge whatwg/whatwg.org#230 because then those styles would mess up quirks.whatwg.org. Probably we should align? :-/

@domenic
Copy link
Member

domenic commented Oct 20, 2018

Can we omit these from review draft mode?

@sideshowbarker
Copy link
Member Author

Can we omit these from review draft mode?

I think we already have it doing that — see 6bb32df. Is that not working as expected?

@sideshowbarker
Copy link
Member Author

The outputted HTML here differs from what I'm seeing in e.g. the bottom of https://quirks.spec.whatwg.org/#the-hashless-hex-color-quirk , which I assume is the HTML produced by Bikeshed. Among other things, this means we can't merge whatwg/whatwg.org#230 because then those styles would mess up quirks.whatwg.org. Probably we should align? :-/

OK, to fix that for now, I can update the stylesheet patch to instead use the class name wpt-tests-margin (or whatever — lemme know if you have a suggestion) instead of wpt-tests-block (which is the class name that Bikeshed uses for it’s non-marginal <wpt> output).

Would that be sufficient for now?

Long term, I think @tabatkins has already said he’s supportive of adding support to Bikeshed for also styling the <wpt> output as marginal annotations (at least as a user option) — so I think we can eventually use the same styles.

But for right now, if I just change the class name, that avoids the current conflict. So please lemme know if that would resolve it satisfactorily for the time being.

@domenic
Copy link
Member

domenic commented Oct 21, 2018

Agreed, let's just change the class name. I'll try doing that and merging myself.

We should come up with a nice long-term plan for the markup and CSS for all of this. See whatwg/meta#117 for my thoughts there. But let's get it all in and working first :).

domenic pushed a commit to whatwg/whatwg.org that referenced this pull request Oct 21, 2018
See whatwg/wattsi#90. Eventually we may use similar styles for <wpt> annotations in specs besides HTML; for now they are using Bikeshed's built-in styles. Unification is tracked at whatwg/meta#117.
@domenic domenic merged commit 1512df5 into master Oct 21, 2018
@domenic domenic deleted the sideshowbarker/wpt-element branch October 21, 2018 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants