Add ability to show code with line numbers #14

allejo · 2019-10-04T07:03:18Z

I haven't developed for WordPress in years, I'm hoping I'm not too far off with this PR.

How do you want to handle line numbers on the front-end?

Should this plug-in introduce its own CSS dependency to add numbers via pseudo-elements?
Should it create a table like GitHub?
Should showing line numbers be a global configuration instead of just a per-block basis?

Closes #11

Edit: Looks like the build failed. Should .editorconfig enforce tabs for indentation? I was hoping my IDE would pick up the correct indentation but it doesn't look like it did.

westonruter · 2019-10-04T16:35:51Z

I haven't developed for WordPress in years, I'm hoping I'm not too far off with this PR.

Thank you!

How do you want to handle line numbers on the front-end?

I didn't have a specific solution in mind.

Should this plug-in introduce its own CSS dependency to add numbers via pseudo-elements?

Yes, perhaps using GitHub's strategy?

But…

Should it create a table like GitHub?

I'm not a huge fan of using tables for this. Seems like a hack. I wonder if lines could be styled with numbers via display:list-item and list-style-type:decimal?

Should showing line numbers be a global configuration instead of just a per-block basis?

I think it's good to be per-block basis.

allejo · 2019-10-05T04:51:06Z

I'm not a huge fan of using tables for this. Seems like a hack. I wonder if lines could be styled with numbers via display:list-item and list-style-type:decimal?

👍 for not using tables for this.

I tried giving display: list-item a try, but it didn't work out too well because I couldn't get the difference in digits to line up neatly nor get rid of the periods.

Currently, I'm using a CSS counter for the line numbers, but I can get the number generation happening on the PHP side and use attr() for the content instead of counter().

Thoughts?

westonruter

For counter(), what happens if you have two code blocks one after the other? Does it reset?

syntax-highlighting-code-block.php

westonruter · 2019-10-07T01:35:17Z

syntax-highlighting-code-block.php

+			// We need to wrap the line of code twice in order to let out `white-space: pre` CSS setting to be respected
+			// by our `table-row`.
+			foreach ( $lines as $line ) {
+				$code .= sprintf( '<div class="loc"><span>%s</span></div>%s', $line, PHP_EOL );


In the future it will be useful to include the loc even if line numbers aren't being displayed. This will make it easy to highlight lines.

Are you okay with adding the extra overhead on the initial render of splitting up code into lines even if it's not showing line numbers or highlighting lines?

I think so. How much overhead is it?

It's nothing too crazy:

In splitCodeIntoArray() from highlight.php:

It'll parse the highlighted HTML into a \DOMDocument

Loop through all of the spans and see if there are any new lines

If new lines exist, modify the lines of code to add closing and opening tags

Split the modified code into an array.

In this plug-in:

Loop through all of the lines of code and wrap them.

I wonder. Why is splitCodeIntoArray even needing to load the string into a DOMDocument to do this? Why can't it just split the string by lines given that it is inside of a pre?

Parsing it as a DOM is necessary, take this snippet as an example:

/** * Hello World * * @api * @since 1.0.0 * @param string $str Some string parameter */

The way highlight.js works (and in turn, highlight.php, since it's a strict port), you would get this HTML:

<span class="hljs-comment">/** * Hello World * * <span class="hljs-doctag">@api</span> * <span class="hljs-doctag">@since</span> 1.0.0 * <span class="hljs-doctag">@param</span> string $str Some string parameter */</span>

If you split up by newlines, you'd break <span>s that have new lines in them. You'd get something like:

<div class="loc"><span><span class="hljs-comment">/**</span></div> <div class="loc"><span> * Hello World</span></div> <div class="loc"><span> *</span></div> <div class="loc"><span> * <span class="hljs-doctag">@api</span></span></div> <div class="loc"><span> * <span class="hljs-doctag">@since</span> 1.0.0</span></div> <div class="loc"><span> * <span class="hljs-doctag">@param</span> string $str Some string parameter</span></div> <div class="loc"><span> */</span></span></div>

If there's a better and robust solution to splitting into lines without parsing it, I'm all ears! 😅

Good observation. I don't have any better recommendation!

index.css

westonruter

Nice work on this!

westonruter · 2019-12-15T14:59:54Z

syntax-highlighting-code-block.php

+	wp_enqueue_style( FRONTEND_HIGHLIGHT_STYLE_HANDLE );
+	wp_add_inline_style(
+		FRONTEND_HIGHLIGHT_STYLE_HANDLE,
+		file_get_contents( plugins_url( 'index.css', __FILE__ ) ) // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents


This is fetching the file over HTTP instead of getting it from the filesystem. It should have been:

file_get_contents( __DIR__ . '/line-numbers.css' ) // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents

See #25/#26

Ah good to know for future reference. Thanks for fixing it!

allejo added 3 commits October 4, 2019 12:54

Add ability to show code w/line numbers

2ef0db2

Add CSS displaying line numbers

07ca6ba

Explicitly specify tab indentation in editorconfig

e0be9f3

allejo marked this pull request as ready for review October 5, 2019 04:43

westonruter reviewed Oct 7, 2019

View reviewed changes

allejo commented Oct 7, 2019

View reviewed changes

index.css Show resolved Hide resolved

Remove extra external stylesheet for this plugin

8ab0b66

allejo requested a review from westonruter October 11, 2019 22:19

westonruter approved these changes Oct 12, 2019

View reviewed changes

westonruter merged commit 9dfe557 into westonruter:master Oct 12, 2019

allejo deleted the feature/show-line-numbers branch October 13, 2019 01:33

westonruter added this to the 1.1.0 milestone Nov 22, 2019

westonruter mentioned this pull request Nov 23, 2019

Add missing HighlightUtilities/functions.php to ZIP #23

Closed

westonruter reviewed Dec 15, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to show code with line numbers #14

Add ability to show code with line numbers #14

allejo commented Oct 4, 2019 •

edited

Loading

westonruter commented Oct 4, 2019

allejo commented Oct 5, 2019

westonruter left a comment

westonruter Oct 7, 2019

allejo Oct 7, 2019

westonruter Oct 7, 2019

allejo Oct 7, 2019

westonruter Oct 8, 2019

allejo Oct 8, 2019

allejo Oct 8, 2019

westonruter Oct 12, 2019

westonruter left a comment

westonruter Dec 15, 2019 •

edited

Loading

allejo Dec 15, 2019

Add ability to show code with line numbers #14

Add ability to show code with line numbers #14

Conversation

allejo commented Oct 4, 2019 • edited Loading

westonruter commented Oct 4, 2019

allejo commented Oct 5, 2019

westonruter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter left a comment

Choose a reason for hiding this comment

westonruter Dec 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allejo commented Oct 4, 2019 •

edited

Loading

westonruter Dec 15, 2019 •

edited

Loading