Line highlighting not working with WP 4.2.2

[nurtext]

Used: [gist id="..." highlight="1" highlight_color="#ffffcc"]

Rendered HTML: https://gist.github.com/nurtext/1642405725de2fa8ed3f

As far as I can see no inline-style attribute is set as it should be…

[GaryJones]

WP 4.3 also looks to be the same, not showing highlighting.

[nurtext]

I investigated further: I seems GitHub has changed the HTML code from div and spans to table rows and cells.

[GaryJones]

Thanks for the pointer - definitely one to look into.

[nurtext]

My changes for class-gistpress.php in line 623-626 - new regex to hide line numbers.

// Remove the line number cell if it has been disabled.
if ( ! $args['show_line_numbers'] ) {
    $html = preg_replace( '#<td.*data-line-number.*></td>#s', '', $html );
}

Haven't had time go get further into this, maybe at the weekend. :)

[GaryJones]

Feel free to do proper PR(s), so then you can get proper attribution :-)

[nurtext]

Sure, I already have some working changes going on and running tests in my blog. Btw: forget about the regex above, its failing ;)

Will submit some PRs later

[arm4b]

I can confirm that not only highlighting, but also show_line_numbers= and showing specific lines with lines= and lines_start= is broken, see this screenshot:

I've discovered it via code mess appeared on our Wordpress production.

---

Definitely looks like upstream GitHub changes. Probably it's critical issue, because big part of plugin functionality is broken now.

[GaryJones]

Probably it's critical issue, because big part of plugin functionality is broken now.

Agreed. I'll add this to my list for this week.

[bradyvercher]

@GaryJones I had a little bit of time to look into this over the weekend, so gave it a quick go. I think it should fix up any issues related to the structure change, but it could use another pair of eyes if you get a chance to review it.

[GaryJones]

@bradyvercher Thanks - I'll take a look.

As an aside, it was my plan, along with #38, to pull all of the regexes into either their own class, or as const's for the class they are used in, to make it easier to find and change in the future.

One thing to watch out for, is that we should consider deleting the transients on plugin update, so that div/pre based stored HTML is wiped out and the new table-based HTML can be stored instead, to avoid old markup being handled with the newer plugin.

[bradyvercher]

I was mainly looking to pitch in and help put together a quick fix. Abstracting the regexes and cleaning up the classes would be great, though. Feel free to make any improvements you see fit. I won't be able to dig again for a few days, but ping me if you'd like me to look anything over.

[GaryJones]

A couple of minor issues:

Strict Standards: Non-static method DOMDocument::loadHTML() should not be called statically, assuming $this from incompatible context in /srv/www/wordpress-develop/src/wp-content/plugins/gistpress/includes/class-gistpress.php on line 472

Strict Standards: Non-static method DOMDocument::loadHTML() should not be called statically, assuming $this from incompatible context in /srv/www/wordpress-develop/src/wp-content/plugins/gistpress/includes/class-gistpress.php on line 557

lines, show_line_numbers and highlight all seem to be working correctly. lines_start does NOT seem to be working.

[bradyvercher]

Weird. The docs for DOMDocument::loadHTML() say it can be called statically, but if I would have read further down, it does mention throwing a strict notice if so.

lines_start seems to be working fine for me. Maybe it's the combination of parameters your'e using that are making it fail?

[GaryJones]

My bad; lines_start was working fine.

Good to merge into develop, master, tag and release. Getting this bug fix out is more important than cleaning up any code.

[bradyvercher]

These changes have been merged and tagged as 3.0.0 to account for the switch to DOMDocument.

[GaryJones]

It looks like the use of DOMDocument is now putting a <!doctype...> in the middle of the entry content, causing invalid markup.

[Nilpo]

This is the same as issue #63

[GaryJones]

Yep - I know the new issue was coming so made a comment here until it was posted :-)