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

Include last good version in caniuse boxes #39

Merged
merged 5 commits into from
Jan 5, 2017

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Jan 2, 2017

Fixes whatwg/html#2225.


caniuse-mathml

...should probably also change "5.0-5.1-10-10.1" to just "5.0-10.1", but not knowing FreePascal it's not immediately obvious to me how to do so.

@zcorpan
Copy link
Member Author

zcorpan commented Jan 3, 2017

caniuse-mathml-2

Arguably it should be grayed out if it is no longer supported in the last few versions... but maybe this is good enough @bbarenblat?

@bbarenblat
Copy link

I think if a feature is not supported in the latest version of a browser, that browser entry ought to be greyed out. That might be too labour-intensive, though. For instance, what if a feature is supported properly in the Firefox ESR but not in the latest release (due to a regression or dropped support)? The easiest way around this is probably to hook into Can I Use’s usage data and make the grey/black display reflect the most commonly used version. Is there support for that?

I’d also prefer to keep the version+ notation for features that are supported up to the current version, because it doesn’t require readers to keep track of the current versions of every browser to see current support.

@zcorpan
Copy link
Member Author

zcorpan commented Jan 3, 2017

So...

  • If the last good version is the same as the last version, use first good version+
  • If the last good version is not the same as the last version, that means it's no longer supported; gray out the item.

@bbarenblat
Copy link

Yes, that’s right.

@domenic
Copy link
Member

domenic commented Jan 3, 2017

It'd also be ideal to change "11-11" to just "11" while you're here :). Thanks for working on this @zcorpan!!

@zcorpan
Copy link
Member Author

zcorpan commented Jan 4, 2017

caniuse-mathml-3

It seems no other caniuse boxes have anything with "removed".

@Hixie could you maybe review the code?

@zcorpan zcorpan requested a review from Hixie January 4, 2017 08:52
var
i: Integer;
begin
i := Pos(Separator, Txt);
Copy link
Member

Choose a reason for hiding this comment

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

This is expensive. I would use the rope stuff to do a no-copy splitter with a single walk.

Copy link
Member

Choose a reason for hiding this comment

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

Specifically, I'd use the https://github.com/whatwg/wattsi/blob/master/src/lib/stringutils.pas stuff to iterate over the string, grab the start pointer and the middle pointer using a single walk, and return the two CutUTF8Strings.

But whatever. This code would work fine too.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. Could you push a commit to do that? My guess is that it would take me hours to figure out how to use ropes...

Copy link
Member

Choose a reason for hiding this comment

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

Just use whatever works. :-) Maybe drop a link to this review so that someone in the future can improve it if it turns out to be a bottleneck.

@zcorpan
Copy link
Member Author

zcorpan commented Jan 5, 2017

@domenic please merge and do the necessary manual steps (if we still have manual steps). 🙂

@domenic domenic merged commit cf6bb3c into master Jan 5, 2017
@domenic domenic deleted the zcorpan/caniuse-last-version branch January 5, 2017 19:24
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.

4 participants