question: validator-generated.php

[obeyer]

@sebastianbenz @sigginet
I have a question about validator-generated.php
I followed instructions on how to regenerate it by cloning lullabot/amphtml repo and stuff.
I only had one small change (I added amp-video element and the script for it to Context.php). But when I regenerated the file, it was so much different from what it is right now. So it either was changed manually before, and now generated changes are overriding it, or some other changes that people made, required the file to be regenerated, and it wasn't. Also, does that repo https://github.com/Lullabot/amphtml need to be updated as well? looks like the repo it was forked from is so much ahead.
On the side note, I opened a bunch of pull requests, that we had in our version of amp-library, because we decided to keep it up to date with yours, and contribute, since you guys are merging code. Please, look when you have time. Thx!

[renzit]

i can confirm , on a previous pull request , i implement the amp-sidebar tag but i doesnt generate the test and the PR was accepted. If when merged to master they dont recreate the tests it would recreate every time somebody implements something and recreate the validator.

I realize this in a new PR of the amp-accordion when i discover how to make it i do it and its regenerate the amp-sidebar and the amp-accordion , but that isnt merged yet so everybody will have this issue of testing files and regenerate something that they didnt do.

And i also want to know the future of the amphtml library that generates the validator.php, its on plans to keep this update? If yes , i would like to contribute to make this happen :)
Now: This branch is 12 commits ahead, 2160 commits behind ampproject:master.

[renzit]

@obeyer in this PR #162
they are trying to update the validator.php,i think we can help from what they do there.

[luigitec]

I got the following after running composer update:

Warning: Ambiguous class resolution, "Lullabot\AMP\Spec\AmpLayout" was found in both "/Users/.../vendor/lullabot/amp/src/Spec/validato
r-generated.php" and "/Users/.../vendor/lullabot/amp/src/validator-generated.php", the first will be used.

I got this on the require block of the composer.json file:

"lullabot/amp": "^1.0.0",

[obeyer]

@sebastianbenz @sigginet @renzit
so who could merge the pr
to update the code in https://github.com/Lullabot/amphtml with the code from https://github.com/ampproject/amphtml/
at this point the validator is almost a year behind, and we can't use all the new features cretaed in AMP project, since all the new tags get stripped out. is there a point in https://github.com/Lullabot/amphtml at all? can we just make the library to use https://github.com/ampproject/amphtml/ validator? so each time they push new changes (which is pretty often), the Lullabot/amphtml branch doesn't need to be updated?

[sidkshatriya]

The objective of the Lullabot validator is to provide a good fundamental base for HTML->AMP HTML conversion. If you know what validation errors are there, you can try to fix them or flag them to the user.

Yes at this point the Lullabot validator has fallen behind. It would need changes to the codebase to catch up with all the changes to the AMP specification. The Lullabot validator is designed to read the AMP specification written in protobuf so it can deal with incremental changes e.g. a tag was added to a whitelist or a validation regex was changed. But over a period of time there are changes both to the specification structure itself and the AMP specification that (unavoidably) need the Lullabot validator codebase to be modified in step with the canonical Javascript validator.

Unfortunately I don't have the time to bring this upto date with the canonical amphtml validator written in Javascript

[obeyer]

@sidkshatriya
I understand you are not working on this project any more, as you said before. There's a pull request, which I mentioned in my previous comment, sorry didn't put a link to it. Here it is: Lullabot/amphtml#1 by @dawehner
Does it fix the problem, or there's still things that need to be done? In my understanding, new specs also need to be added to the validator-main.protoascii file? what else? I am willing to work on this, but I would appreciate a little guidance. Thank you

[dawehner]

@obeyer I'm happy to answer you answer question on my PR, when I still remember. Just reach out to me somehow. Sadly the test failures in my PR are highly non trivial, at least they weren't at the time. I'm happy to provide help

[westonruter]

I suggest using the generated PHP file in the AMP WordPress plugin: https://github.com/Automattic/amp-wp/blob/develop/includes/sanitizers/class-amp-allowed-tags-generated.php

We'd like to work on extracting this into a Composer package as well to make it easier to incorporate.