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

Upgrade nu.validator html parser #1958

Merged
merged 1 commit into from
Jul 21, 2019

Conversation

nodefactory-bk
Copy link

Liftweb has a slightly ancient nu.validator version.

Specifically it breaks vuetify data-table since it removes tags
inside .

I know I haven't discussed this on the mailing list but since the change is small and passes all lift tests I'm hoping I don't get shot for it =)

Liftweb has a slightly ancient nu.validator version.

Specifically it breaks vuetify data-table since it removes <td> tags
inside <template>.
@Shadowfiend
Copy link
Member

I'm having trouble finding who is publishing these point releases. The original site notes the last release as 1.4, and the Mercurial repo seems to have more recent commits but its last tag is for the 1.4 release. Is it this fork? What does 1.4.12 include with respect to 1.4?

@andreak
Copy link
Member

andreak commented Apr 4, 2019

Why not upgrade to latest from https://github.com/validator/validator ?

@farmdawgnation
Copy link
Member

It looks to be that fork, but they're not tagging releases and may not be pushing the commits that have the version bumps at all? I'm inclined to think this is safe but have very little to go on in terms of validating that this is the same code that's getting released. @sideshowbarker seems to be the maintainer of the fork — maybe he can shed some light on the validity of making this change / dangers thereof?

@sideshowbarker
Copy link

Assuming the releases being referred to here at the maven packages with groupId=nu.validator and artifact=htmlparser:

https://repo1.maven.org/maven2/nu/validator/htmlparser/

…then yeah I am the one doing those releases.

As far as those releases being safe to use: The same code is what I use as the HTML parser for the W3C Nu Html Checker https://validator.w3.org/nu/ — so it’s functioning in that context.

I have not used it myself outside that context, but I don’t have reason to believe it’d have any problems.

One difference from upstream that you should be aware of is that the fork doesn’t expose an XOM API. But that shouldn’t affect you unless (unless of course if you’re actually using the XOM API).

There are some other differences from upstream:

validator/htmlparser@master...validator-nu

…but all the differences there except for the Add .classpath, .settings, .project, .mailmap change are changes to bring the parser into conformance with the current requirements in the HTML spec.

As far as tagging, going forward I can start tagging every time I do a release. The version numbers just reflect that fact I never bump the minor version number (the 1.4) but always only the patch number.

That seems appropriate since the fork introduces no API changes (well, other than completely dropping the XOM API, but I’m not aware of anybody that’s been using that anyway).

@farmdawgnation
Copy link
Member

Awesome, thanks. I think with some certification that these builds are legit, I'm fine to start using it provided things work. The CI build is failing, so I'm not sure what's up with that offhand. Will try to take a look at it before too long.

@Shadowfiend
Copy link
Member

Looks like it was one of our favorite TimeHelpers errors. Restarted the build.

@farmdawgnation farmdawgnation added this to In progress in Lift 3.4.0 via automation Jul 21, 2019
@farmdawgnation farmdawgnation merged commit 195da61 into lift:master Jul 21, 2019
Lift 3.4.0 automation moved this from In progress to Done Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Lift 3.4.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants