Mirroring script: Edge mirrors from both IE AND Chrome at the same time

[queengooborg]

This PR refactors the mirroring script, adding new functions (and a few bug fixes), to mirror data for Edge a little bit differently in light of its engine change. Now, rather than mirroring from just IE or just Chrome, it will mirror the data from both browsers at once.

This includes the addition of a combineStatements() function, which takes an array of support statements and combines them down to their simplest form possible. This will be helpful for future multi-source mirroring to come. Accordingly, this also provides a structure for multi-source mirroring.

Inspired by #6575.

[foolip]

I tried running npm run mirror -- --m always edge (with workaround for a bug in bumpEdge added) and then git diff | grep '^+ ' | sed 's/^+ *//;s/,$//' | sort | uniq -c | sort -nr to see what kinds of changes this is mostly making.

It looks pretty reasonable, but it makes me suspect there are fixes that would improve this a lot. Most of the changes are adding "≤79", and very few "79" are added. There should be lots of APIs the data says was not in IE but was in Chrome ≤79, and where the value should be set to "79". The first cases in the diff of this is AbortController.json. Can you experiment with checking for false in the IE data, if that's not already being done?

Then there are lots of nulls being added. Sometimes that makes sense, if it was added in Edge 12-18 then obviously we can't infer the data from IE+Chrome. There are so many nulls that I suspect that there is still some bug here, but I've failed to find an incorrect change by skimming through the diffs. I can take another look after the PR has been updated.

[foolip]

Sweet stuff! I have a lot of comments, but that's not because it ain't sweet :)

[foolip]

You can turn this into an else if and save some indentation in the following.

[foolip]

@vinyldarkscratch is this ready for final review now? It would be good to first verify that the mirror script still produces exactly the same changes for all other browsers, with/without --source and with/without --modify=always. It would also be good to review a diff of what the mirroring script would do for Edge before and after these changes, to see that there aren't any large regressions.

[foolip]

@vinyldarkscratch can you ping me when you'd like review of this again?

[queengooborg]

This is all good to go now, yep!

[foolip]

Oops, I didn't notice I was pinged for review two weeks ago :)

[foolip]

I tried running npm run mirror -- --modify=always edge but some of the suggested changes don't seem right. For example, in api/AbortController.json, version_added is being set to false, even though it was in Chrome 66. The right answer if ignoring the existing data should be "79". (It not using --modify=always it isn't modified.)

@vinyldarkscratch can you tweak the script until the output when using --modify=always is reasonable when spot checking?

[queengooborg]

This is all ready to go; everything's been double-checked for functionality. I would love to get this PR merged sooner than later, as it paves the way for two new PRs I have planned (one that fixes the WebView mirroring, another that has Opera Android account for data from both Opera Presto and Chrome Android).

[queengooborg]

I just fixed up the logic in regards to those three odd statements, and I have confirmed they don't negatively impact other statements as well!

[foolip]

I've tried npm run mirror -- -m always edge again, now it throws with an error.

It seems like the approach here is very brittle since there are new kinds of errors with each iteration. Can we simplify this to supporting mirroring from IE or Chrome, not both at the same time? In other words, support --source, and have it default to Chrome.

Then it would be straightforward to review the changes that npm run mirror -- --source ie edge would make, and after that we could pretty much forget about that code path.

[queengooborg]

I just pushed a few changes that allow for selecting a source for Edge, and fixed the error I think you might've experienced! I still have the dual browsers selected as the default for mirroring, but a --source argument is now allowed once again, though this time with a validation to ensure it's only one of three options we've scripted.

[github-actions]

This pull request has merge conflicts that must be resolved before we can merge this.

[github-actions]

This pull request has merge conflicts that must be resolved before we can merge this.

[queengooborg]

Made the default as just chrome now actually, with build-time mirroring coming up it's less likely that we'll want to mirror IE data.