Embedded vpatch formatting for mp-wp, draft vpatch for review

After some discussion around what should be included in a first version of this feature I have published for review a draft vpatch of the mp-wp patch viewer plugin. There are several things to call out in the code so let's go through this line-by-line:

#S1-L9: I think this file should be renamed to something like mp-wp-content-processing.php but left it as is for now to provide a meaningful diff for review (rather than the wholesale file deleted / file added you'd get otherwise).

#S1-L22: This is not so much to claim authorship but rather to claim ownershipi and to make it clear that the world starts here.

#S1-L47: These constants were moved inside the class. I couldn't see any reason for them to remain global. The WP_FOOTNOTES_VERSION was removed because its only purpose was to help manage the options state between db and file.

#S1-L63: For obvious reasons, I can't render the codeblock delimiters inside a codeblock delimited by the codeblock delimitersii—which, in this patch, I have set to [[ and /]]. I grepped through the trb genesis and found no matches, and in all 162k! lines of the mp-wp genesisiii there were only three instances of [[ and six of /]]. It appears to be safe for bash as well, at least based on what I could find in the wild. I personally like these delimiters because they are succinct and follow closely the established pattern of (( )) for footnotes.

#S1-L102: This had to come before the wpautop filter which is responsible for inserting <p> and <br /> tags all over the place. Thankfully wpautop ignores content within <pre> tags so that its ill effects can be avoided by processing the codeblocks before it has a chance to runiv. The priority for footnotes processing was left unchanged.

Known limitations: Some stress testing that I did locally revealed an upper limit for a single snippet of approximately 5k lines of code. Adding multiple ~5k snippets to a single article topped out around roughly ~20k lines (4 snippets). Testing with ~525 line snippets failed somewhere between 55-60 snippets on the page (so getting really close to 30k lines there before bumping into the 128MB default memory limit for a PHP script). I have not yet tried these tests with an increased memory limit since the results seemed more than sufficient for how it would be used.

That covers the bulk of it, the rest should be self-explanatory. In terms of overall design, I did think of combining the footnotes and codeblock processing passes into a single filter but we would have to get around the issue of wpautop (codeblocks coming before and footnotes after). However, I couldn't see a performance justification for this since the extraction -> formatting -> reinsertion loops would still need to happen in separate passes.

The patch and signature for anyone who would like to take it for a spin in the own environment:

mp-wp_apply-htmlspecialchars-to-post-edit-content.vpatch
mp-wp_apply-htmlspecialchars-to-post-edit-content.vpatch.billymg.sig

1 diff -uNr a/mp-wp/manifest b/mp-wp/manifest
2 --- a/mp-wp/manifest 4ad5c0b7eda9c670f311a23da92114ab10bf30b9990b46882047aea3ab569395f643cb4aa6144ee0ba74b9821df1e69b26d299c1f0e9c11631a78c46f95913bd
3 +++ b/mp-wp/manifest e70855d060aefbd26f51cf60e39e553e7666f8f76d49adeb95999ab34401f1984e808082f7f34690ebc31fb93f5da192aa0d74b7f5eabd3e62fae3f2054720f4
4 @@ -5,3 +5,4 @@
5 569483 mp-wp_remove-tinymce-and-other-crud billymg Remove tinymce, most of the importers, the self-update feature, and the google gears and press-this plugins
6 602064 mp-wp_apply-htmlspecialchars-to-post-edit-content billymg Run post content through htmlspecialchars() before loading into the post edit UI
7 605926 mp-wp_comments_filtering diana_coman Recent comments widget should show only people's comments (no track/pingbacks); theme default changed to show trackbacks/pingbacks as last/at the bottom in an article's comments list.
8 +614805 mp-wp_add-embedded-vpatch-formatting billymg Add the ability to embed vpatches within article content. Embedded vpatch blocks will be formatted with diff syntax highlighting and anchored line numbers
9 diff -uNr a/mp-wp/wp-content/plugins/footnotes.php b/mp-wp/wp-content/plugins/footnotes.php
10 --- a/mp-wp/wp-content/plugins/footnotes.php 8e2449d4ac26ea05f080cec9d025ef8a8585221ee30da439b37ff1578d084e1c63cbe3f89e3d6868c19d0fa9f73a9af99b444251e7a854f6c87e316628d94859
11 +++ b/mp-wp/wp-content/plugins/footnotes.php 1109723713df46f7ec135f0640d3acba475974f439d722b379581dd262b5f7bf42295ca1e57600d6bb9636a2e9d35965d3841d74fc15d5bf3952cfada98f363b
12 @@ -1,49 +1,28 @@
13 <?php
14 /*
15 -Plugin Name: WP-Footnotes
16 -Plugin URI: http://www.elvery.net/drzax/more-things/wordpress-footnotes-plugin/
17 -Version: 4.2
18 -Description: Allows a user to easily add footnotes to a post.
19 -Author: Simon Elvery
20 -Author URI: http://www.elvery.net/drzax/
21 +Plugin Name: MP-WP-Content-Processing
22 +Plugin URI: http://billymg.com/category/mp-wp/
23 +Description: Allows for the custom processing of article content. Currently supports footnotes and embedded vpatch snippets.
24 +Author: billymg
25 +Author URI: http://billymg.com
26 */
27
28 -/*
29 - * This file is part of WP-Footnotes a plugin for Word Press
30 - * Copyright (C) 2007 Simon Elvery
31 - *
32 - * This program is free software; you can redistribute it and/or
33 - * modify it under the terms of the GNU General Public License
34 - * as published by the Free Software Foundation; either version 2
35 - * of the License, or (at your option) any later version.
36 - *
37 - * This program is distributed in the hope that it will be useful,
38 - * but WITHOUT ANY WARRANTY; without even the implied warranty of
39 - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
40 - * GNU General Public License for more details.
41 - *
42 - * You should have received a copy of the GNU General Public License
43 - * along with this program; if not, write to the Free Software
44 - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
45 - */
46 -
47 -// Some important constants
48 -define('WP_FOOTNOTES_OPEN', " (("); //You can change this if you really have to, but I wouldn't recommend it.
49 -define('WP_FOOTNOTES_CLOSE', "))"); //Same with this one.
50 -define('WP_FOOTNOTES_VERSION', '4.2');
51 -
52 // Instantiate the class
53 -$swas_wp_footnotes = new swas_wp_footnotes();
54 +$mp_wp_content_processing = new mp_wp_content_processing();
55
56 // Encapsulate in a class
57 -class swas_wp_footnotes {
58 - var $current_options;
59 - var $default_options;
60 +class mp_wp_content_processing {
61 + const MP_WP_FOOTNOTES_OPEN = " ((";
62 + const MP_WP_FOOTNOTES_CLOSE = "))";
63 + const MP_WP_CODEBLOCK_OPEN = "NOT DISPLAYED HERE, SEE ARTICLE";
64 + const MP_WP_CODEBLOCK_CLOSE = "NOT DISPLAYED HERE, SEE ARTICLE";
65 +
66 + var $options;
67
68 /**
69 * Constructor.
70 */
71 - function swas_wp_footnotes() {
72 + function mp_wp_content_processing() {
73 // Define the implemented option styles
74 $this->styles = array(
75 'decimal' => '1,2...10',
76 @@ -56,17 +35,16 @@
77 );
78
79 // Define default options
80 - $this->default_options = array('superscript'=>true,
81 + $this->options = array('superscript'=>true,
82 'pre_backlink'=>' [',
83 'backlink'=>'&#8617;',
84 'post_backlink'=>']',
85 'pre_identifier'=>'',
86 - 'list_style_type'=>'decimal',
87 + 'list_style_type'=>'lower-roman',
88 'list_style_symbol'=>'&dagger;',
89 'post_identifier'=>'',
90 'pre_footnotes'=>'',
91 'post_footnotes'=>'',
92 - 'style_rules'=>'ol.footnotes{font-size:0.8em; color:#666666;}',
93 'no_display_home'=>false,
94 'no_display_archive'=>false,
95 'no_display_date'=>false,
96 @@ -74,58 +52,80 @@
97 'no_display_search'=>false,
98 'no_display_feed'=>false,
99 'combine_identical_notes'=>false,
100 - 'priority'=>11,
101 - 'version'=>WP_FOOTNOTES_VERSION);
102 + 'codeblocks_priority'=>9, // highest value that comes before wpautop filter
103 + 'footnotes_priority'=>11);
104
105 - // Get the current settings or setup some defaults if needed
106 - if (!$this->current_options = get_option('swas_footnote_options')){
107 - $this->current_options = $this->default_options;
108 - update_option('swas_footnote_options', $this->current_options);
109 - } else {
110 - // Set any unset options
111 - if ($this->current_options['version'] != WP_FOOTNOTES_VERSION) {
112 - foreach ($this->default_options as $key => $value) {
113 - if (!isset($this->current_options[$key])) {
114 - $this->current_options[$key] = $value;
115 - }
116 - }
117 - $this->current_options['version'] = WP_FOOTNOTES_VERSION;
118 - update_option('swas_footnote_options', $this->current_options);
119 - }
120 + // Hook me up
121 + add_action('the_content', array($this, 'process_codeblocks'), $this->options['codeblocks_priority']);
122 + add_action('the_content', array($this, 'process_footnotes'), $this->options['footnotes_priority']);
123 + add_action('wp_head', array($this, 'insert_styles'));
124 + }
125 +
126 + /**
127 + * Searches the text and apply markup to codeblocks.
128 + * Adds line number links and diff syntax highlighting.
129 + * @param $data string The content of the post.
130 + * @return string The new content with formatted codeblocks.
131 + */
132 + function process_codeblocks($data) {
133 + global $post;
134 +
135 + // Regex extraction of all codeblocks (or return if there are none)
136 + if (
137 + !preg_match_all(
138 + "/(".preg_quote(self::MP_WP_CODEBLOCK_OPEN).")(.*)(".preg_quote(self::MP_WP_CODEBLOCK_CLOSE, '/').")/Us",
139 + $data,
140 + $codeblocks,
141 + PREG_SET_ORDER
142 + )
143 + ) {
144 + return $data;
145 }
146
147 -/*
148 - if (!empty($_POST['save_options'])){
149 - $footnotes_options['superscript'] = (array_key_exists('superscript', $_POST)) ? true : false;
150 - $footnotes_options['pre_backlink'] = $_POST['pre_backlink'];
151 - $footnotes_options['backlink'] = $_POST['backlink'];
152 - $footnotes_options['post_backlink'] = $_POST['post_backlink'];
153 - $footnotes_options['pre_identifier'] = $_POST['pre_identifier'];
154 - $footnotes_options['list_style_type'] = $_POST['list_style_type'];
155 - $footnotes_options['post_identifier'] = $_POST['post_identifier'];
156 - $footnotes_options['list_style_symbol'] = $_POST['list_style_symbol'];
157 - $footnotes_options['pre_footnotes'] = stripslashes($_POST['pre_footnotes']);
158 - $footnotes_options['post_footnotes'] = stripslashes($_POST['post_footnotes']);
159 - $footnotes_options['style_rules'] = stripslashes($_POST['style_rules']);
160 - $footnotes_options['no_display_home'] = (array_key_exists('no_display_home', $_POST)) ? true : false;
161 - $footnotes_options['no_display_archive'] = (array_key_exists('no_display_archive', $_POST)) ? true : false;
162 - $footnotes_options['no_display_date'] = (array_key_exists('no_display_date', $_POST)) ? true : false;
163 - $footnotes_options['no_display_category'] = (array_key_exists('no_display_category', $_POST)) ? true : false;
164 - $footnotes_options['no_display_search'] = (array_key_exists('no_display_search', $_POST)) ? true : false;
165 - $footnotes_options['no_display_feed'] = (array_key_exists('no_display_feed', $_POST)) ? true : false;
166 - $footnotes_options['combine_identical_notes'] = (array_key_exists('combine_identical_notes', $_POST)) ? true : false;
167 - $footnotes_options['priority'] = $_POST['priority'];
168 - update_option('swas_footnote_options', $footnotes_options);
169 - }elseif(!empty($_POST['reset_options'])){
170 - update_option('swas_footnote_options', '');
171 - update_option('swas_footnote_options', $this->default_options);
172 + for ($i = 0; $i < count($codeblocks); $i++) {
173 + $codeblocks[$i]['snippet'] = $this->format_snippet($codeblocks[$i][2], $i+1);
174 }
175 -*/
176
177 - // Hook me up
178 - add_action('the_content', array($this, 'process'), $this->current_options['priority']);
179 - add_action('admin_menu', array($this, 'add_options_page')); // Insert the Admin panel.
180 - add_action('wp_head', array($this, 'insert_styles'));
181 + foreach ($codeblocks as $key => $value) {
182 + $data = substr_replace($data, $value['snippet'], strpos($data,$value[0]),strlen($value[0]));
183 + }
184 +
185 + return $data;
186 + }
187 +
188 + function format_snippet($snippet, $snippet_number) {
189 + $formatted_snippet = htmlspecialchars($snippet);
190 + $code_lines = explode("\r\n", $formatted_snippet);
191 +
192 + foreach ($code_lines as $idx => $line) {
193 + $line_number = sprintf('S%d-L%d', $snippet_number, $idx+1);
194 + $line_link = sprintf('<a href="#%s" name="%s">%d</a>', $line_number, $line_number, $idx+1);
195 + $line_open = sprintf('<tr><td class="line-number-column">%s</td><td class="content-column">', $line_link);
196 + $line_close = '</td></tr>';
197 +
198 + if (substr($line, 0, 5) == 'diff ') {
199 + $code_lines[$idx] = sprintf('%s<span class="line-filename">%s</span>%s', $line_open, $line, $line_close);
200 + } elseif (substr($line, 0, 4) == '--- ' || substr($line, 0, 4) == '+++ ' || substr($line, 0, 3) == '@@ ') {
201 + $code_lines[$idx] = sprintf('%s<span class="line-meta">%s</span>%s', $line_open, $line, $line_close);
202 + } elseif (substr($line, 0, 1) == '-') {
203 + $code_lines[$idx] = sprintf('%s<span class="line-removed">%s</span>%s', $line_open, $line, $line_close);
204 + } elseif (substr($line, 0, 1) == '+') {
205 + $code_lines[$idx] = sprintf('%s<span class="line-added">%s</span>%s', $line_open, $line, $line_close);
206 + } else {
207 + $code_lines[$idx] = sprintf('%s<span class="line-default">%s</span>%s', $line_open, $line, $line_close);
208 + }
209 + }
210 +
211 + $formatted_snippet = implode("\n", $code_lines);
212 +
213 + $formatted_snippet = sprintf(
214 + '%s%s%s',
215 + '<pre class="mp-wp-codeblock"><table cellpadding="0" cellspacing="0"><tbody>',
216 + $formatted_snippet,
217 + '</tbody></table></pre>'
218 + );
219 +
220 + return $formatted_snippet;
221 }
222
223 /**
224 @@ -134,25 +134,28 @@
225 * @param $data string The content of the post.
226 * @return string The new content with footnotes generated.
227 */
228 - function process($data) {
229 + function process_footnotes($data) {
230 global $post;
231
232 // Check for and setup the starting number
233 $start_number = (preg_match("|<!\-\-startnum=(\d+)\-\->|",$data,$start_number_array)==1) ? $start_number_array[1] : 1;
234
235 - // Regex extraction of all footnotes (or return if there are none)
236 - if (!preg_match_all("/(".preg_quote(WP_FOOTNOTES_OPEN)."|<footnote>)(.*)(".preg_quote(WP_FOOTNOTES_CLOSE)."|<\/footnote>)/Us", $data, $identifiers, PREG_SET_ORDER)) {
237 + // Remove codeblocks from content to be parsed for footnotes
238 + $data_sans_codeblocks = preg_replace("/(<pre class=\"mp-wp-codeblock\">)(.*)(<\/pre>)/Us", '', $data);
239 +
240 + // Regex extraction of all footnotes from non-codeblock content (or return if there are none)
241 + if (!preg_match_all("/(".preg_quote(self::MP_WP_FOOTNOTES_OPEN)."|<footnote>)(.*)(".preg_quote(self::MP_WP_FOOTNOTES_CLOSE)."|<\/footnote>)/Us", $data_sans_codeblocks, $identifiers, PREG_SET_ORDER)) {
242 return $data;
243 }
244
245 // Check whether we are displaying them or not
246 $display = true;
247 - if ($this->current_options['no_display_home'] && is_home()) $display = false;
248 - if ($this->current_options['no_display_archive'] && is_archive()) $display = false;
249 - if ($this->current_options['no_display_date'] && is_date()) $display = false;
250 - if ($this->current_options['no_display_category'] && is_category()) $display = false;
251 - if ($this->current_options['no_display_search'] && is_search()) $display = false;
252 - if ($this->current_options['no_display_feed'] && is_feed()) $display = false;
253 + if ($this->options['no_display_home'] && is_home()) $display = false;
254 + if ($this->options['no_display_archive'] && is_archive()) $display = false;
255 + if ($this->options['no_display_date'] && is_date()) $display = false;
256 + if ($this->options['no_display_category'] && is_category()) $display = false;
257 + if ($this->options['no_display_search'] && is_search()) $display = false;
258 + if ($this->options['no_display_feed'] && is_feed()) $display = false;
259
260 $footnotes = array();
261
262 @@ -160,7 +163,7 @@
263 if ( array_key_exists(get_post_meta($post->ID, 'footnote_style', true), $this->styles) ) {
264 $style = get_post_meta($post->ID, 'footnote_style', true);
265 } else {
266 - $style = $this->current_options['list_style_type'];
267 + $style = $this->options['list_style_type'];
268 }
269
270 // Create 'em
271 @@ -175,7 +178,7 @@
272
273
274 // if we're combining identical notes check if we've already got one like this & record keys
275 - if ($this->current_options['combine_identical_notes']){
276 + if ($this->options['combine_identical_notes']){
277 for ($j=0; $j<count($footnotes); $j++){
278 if ($footnotes[$j]['text'] == $identifiers[$i]['text']){
279 $identifiers[$i]['use_footnote'] = $j;
280 @@ -206,12 +209,9 @@
281 $id_id = "identifier_".$key."_".$post->ID;
282 $id_num = ($style == 'decimal') ? $value['use_footnote']+$start_number : $this->convert_num($value['use_footnote']+$start_number, $style, count($footnotes));
283 $id_href = ( ($use_full_link) ? get_permalink($post->ID) : '' ) . "#footnote_".$value['use_footnote']."_".$post->ID;
284 -
285 -// $id_title = str_replace('"', "&quot;", htmlentities(strip_tags($value['text']), ENT_QUOTES, 'UTF-8'));
286 -
287 $id_title = str_replace('"', '`', strip_tags($value['text']));
288 - $id_replace = $this->current_options['pre_identifier'].'<a href="'.$id_href.'" id="'.$id_id.'" class="footnote-link footnote-identifier-link" title="'.$id_title.'">'.$id_num.'</a>'.$this->current_options['post_identifier'];
289 - if ($this->current_options['superscript']) $id_replace = '<sup>'.$id_replace.'</sup>';
290 + $id_replace = $this->options['pre_identifier'].'<a href="'.$id_href.'" id="'.$id_id.'" class="footnote-link footnote-identifier-link" title="'.$id_title.'">'.$id_num.'</a>'.$this->options['post_identifier'];
291 + if ($this->options['superscript']) $id_replace = '<sup>'.$id_replace.'</sup>';
292 if ($display) $data = substr_replace($data, $id_replace, strpos($data,$value[0]),strlen($value[0]));
293 else $data = substr_replace($data, '', strpos($data,$value[0]),strlen($value[0]));
294 }
295 @@ -219,14 +219,14 @@
296 // Display footnotes
297 if ($display) {
298 $start = ($start_number != 1) ? 'start="'.$start_number.'" ' : '';
299 - $data = $data.$this->current_options['pre_footnotes'];
300 + $data = $data.$this->options['pre_footnotes'];
301
302 $data = $data . '<ol '.$start.'class="footnotes">';
303 foreach ($footnotes as $key => $value) {
304 $data = $data.'<li id="footnote_'.$key.'_'.$post->ID.'" class="footnote"';
305 if ($style == 'symbol') {
306 $data = $data . ' style="list-style-type:none;"';
307 - } elseif($style != $this->current_options['list_style_type']) {
308 + } elseif($style != $this->options['list_style_type']) {
309 $data = $data . ' style="list-style-type:' . $style . ';"';
310 }
311 $data = $data . '>';
312 @@ -236,56 +236,39 @@
313 $data = $data.$value['text'];
314 if (!is_feed()){
315 foreach($value['identifiers'] as $identifier){
316 - $data = $data.$this->current_options['pre_backlink'].'<a href="'.( ($use_full_link) ? get_permalink($post->ID) : '' ).'#identifier_'.$identifier.'_'.$post->ID.'" class="footnote-link footnote-back-link">'.$this->current_options['backlink'].'</a>'.$this->current_options['post_backlink'];
317 + $data = $data.$this->options['pre_backlink'].'<a href="'.( ($use_full_link) ? get_permalink($post->ID) : '' ).'#identifier_'.$identifier.'_'.$post->ID.'" class="footnote-link footnote-back-link">'.$this->options['backlink'].'</a>'.$this->options['post_backlink'];
318 }
319 }
320 $data = $data . '</li>';
321 }
322 - $data = $data . '</ol>' . $this->current_options['post_footnotes'];
323 + $data = $data . '</ol>' . $this->options['post_footnotes'];
324 }
325 return $data;
326 }
327
328 - /**
329 - * Really insert the options page.
330 - */
331 - function footnotes_options_page() {
332 - $this->current_options = get_option('swas_footnote_options');
333 - foreach ($this->current_options as $key=>$setting) {
334 - $new_setting[$key] = htmlentities($setting);
335 - }
336 - $this->current_options = $new_setting;
337 - unset($new_setting);
338 - include (dirname(__FILE__) . '/options.php');
339 - }
340 -
341 - /**
342 - * Insert the options page into the admin area.
343 - */
344 - function add_options_page() {
345 - // Add a new menu under Options:
346 - add_options_page('Footnotes', 'Footnotes', 8, __FILE__, array($this, 'footnotes_options_page'));
347 - }
348 -
349 - function upgrade_post($data){
350 - $data = str_replace('<footnote>',WP_FOOTNOTES_OPEN,$data);
351 - $data = str_replace('</footnote>',WP_FOOTNOTES_CLOSE,$data);
352 - return $data;
353 - }
354 -
355 - function insert_styles(){
356 + function insert_styles() {
357 ?>
358 <style type="text/css">
359 - <?php if ($this->current_options['list_style_type'] != 'symbol'): ?>
360 - ol.footnotes li {list-style-type:<?php echo $this->current_options['list_style_type']; ?>;}
361 + ol.footnotes { font-size: 0.8em; color: #666666; }
362 + pre.mp-wp-codeblock { background: none; color: #333; border: 1px solid #ddd; padding: 0; }
363 + td.line-number-column { background: #f5f6f7; text-align: right; }
364 + td.line-number-column a { color: #555; padding: 0 5px; }
365 + td.content-column { padding-left: 10px; }
366 + span.line-filename { font-weight: bold; }
367 + span.line-meta { color: #999; }
368 + span.line-added { color: green; }
369 + span.line-removed { color:red; }
370 +
371 + <?php if ($this->options['list_style_type'] != 'symbol'): ?>
372 + ol.footnotes li { list-style-type: <?php echo $this->options['list_style_type']; ?>; }
373 <?php endif; ?>
374 - <?php echo $this->current_options['style_rules'];?>
375 +
376 </style>
377 <?php
378 }
379
380
381 - function convert_num ($num, $style, $total){
382 + function convert_num ($num, $style, $total) {
383 switch ($style) {
384 case 'decimal-leading-zero' :
385 $width = max(2, strlen($total));
386 @@ -301,7 +284,7 @@
387 case 'symbol' :
388 $sym = '';
389 for ($i = 0; $i<$num; $i++) {
390 - $sym .= $this->current_options['list_style_symbol'];
391 + $sym .= $this->options['list_style_symbol'];
392 }
393 return $sym;
394 }
395 @@ -318,7 +301,7 @@
396 * @param string $case Upper or lower case.
397 * @return string The roman numeral
398 */
399 - function roman($num, $case= 'upper'){
400 + function roman($num, $case= 'upper') {
401 $num = (int) $num;
402 $conversion = array('M'=>1000, 'CM'=>900, 'D'=>500, 'CD'=>400, 'C'=>100, 'XC'=>90, 'L'=>50, 'XL'=>40, 'X'=>10, 'IX'=>9, 'V'=>5, 'IV'=>4, 'I'=>1);
403 $roman = '';
404 @@ -331,7 +314,7 @@
405 return ($case == 'lower') ? strtolower($roman) : $roman;
406 }
407
408 - function alpha($num, $case='upper'){
409 + function alpha($num, $case='upper') {
410 $j = 1;
411 for ($i = 'A'; $i <= 'ZZ'; $i++){
412 if ($j == $num){
413

———

  1. So that if something goes wrong I'm on the line to fix it, lest my reputation suffer the negratings. []
  2. I tried escaping with &lsqb; and &rsqb; (as I did for this article) but because the code snippet itself is sent through htmlspecialchars that wouldn't work either, so I decided this to be an unwinnable battle and moved on. If anyone would like to point out what I'm missing I'd be happy to update this article. []
  3. 23k of which have since been snipped. []
  4. wpautop can also be disabled at the theme level via the addition of remove_filter( 'the_content', 'wpautop' ); in the theme's function.php file. []

7 Responses to “Embedded vpatch formatting for mp-wp, draft vpatch for review”

  1. Jacob Welsh says:

    1. If I only looked at the manifest entry or even your article title and non-code content here, I'd have no idea that the patch also changes footnote behavior. Not saying the change shouldn't be done, just properly described. For instance, I've noticed various mp-wp blogs use the presently default decimal footnote markers while you're changing it here to roman. Now I dunno, maybe that's another of those things that shouldn't be optional anyway.

    2. Since double-paren footnotes are officially non-optional now, I'd rather it go all the way and remove those WP_FOOTNOTES_OPEN variables that would falsely suggest otherwise, and the sharp edge of that broken preg_quote. Oh hm, you *did* include the slash-quoting for MP_WP_CODEBLOCK_CLOSE but left it out for the other three - why the special case? And why the special case of using a slash in the chosen delimiter too?

    3. Merciful Zeus, horizontal scrolling? And since the scrollbar is buried pages down at the bottom, the only way I can read this at all is with the middle-click autoscroll bubble, and that's still rather a pain. See also and feel free to try the css I used there (inline white-space:pre-wrap).

    4. Just a thought, I'd personally like to see custom syntax highlighting at some point for non-vpatch snippets. This would need a way to specify language though so perhaps this bracket tag isn't the right thing to built it on.

    5. Bash definitely uses double-brackets as builtin operators. Is that why you used the / for the closing?

    6. "but because the code snippet itself is sent through htmlspecialchars that wouldn't work either" - hm, might this be a case of "so don't do that"? Special chars already had to be escaped by the author, in code as elsewhere, so for the serious code-blogger, a sed script that deals with brackets, parens, less-thans and ampersands in one shot seems no worse a workflow than before. Though I get the desire to minimize friction.

  2. billymg says:

    > the patch also changes footnote behavior... For instance, I've noticed various mp-wp blogs use the presently default decimal footnote markers while you're changing it here to roman.

    D'oh, I forgot to call this out. I meant leave a not about it in the article. I used to have decimal identifiers on this blog as well, I changed to lower-case roman numerals because that is what's on Trilema.

    > Oh hm, you *did* include the slash-quoting for MP_WP_CODEBLOCK_CLOSE but left it out for the other three - why the special case?

    The other three, `[[`, `((`, and `))`, don't contain a slash. The slash was added to the codeblock close because `]]` by itself would be matched in bash scripts.

    > Merciful Zeus, horizontal scrolling?

    The horizontal scrolling is something I didn't think much of because I was using a trackpad (scrolls left/right as easily as up/down without looking for the scrollbar). You can also hold shift while scrolling with a mouse scroll wheel to move horizontally.

    Applying `white-space:pre-wrap` and max-width on the code worked but I also think the code itself should be formatted to not have such long lines in the first place. I'm not entirely sure the plugin's default should be to wrap your lines of code.

    What I've seen on a number of republican blogs is exactly this, a fixed width code box with horizontal scrolling. Is this perfect? Probably not but it avoids text lines breaking out of the layout and causing the entire page to horizontally scroll, or machine-wrapping the lines, which can be just as unpleasant to read. That being said, the CSS is customizable and a user can choose to style it however they like.

    > Just a thought, I'd personally like to see custom syntax highlighting at some point for non-vpatch snippets. This would need a way to specify language though so perhaps this bracket tag isn't the right thing to built it on.

    I agree although I don't have any good suggestions off the top of my head for what the delimiters would be. I've seen things like [[python]] ... [[/python]] before so perhaps something like that. MP suggested hijacking html's <code> but I didn't want to do that here because I rely on <code> for inline styling of variables and such.

    > Bash definitely uses double-brackets as builtin operators. Is that why you used the / for the closing?

    Yup, exactly it.

    > Though I get the desire to minimize friction.

    Yes and the fact that literally the only awkward case I came across was the one in this article with code containing the code delimiters themselves. This hardly seems like an edge case worthy of requiring sed pre-processing for anyone wanting to post html-containing snippets.

  3. > Now I dunno, maybe that's another of those things that shouldn't be optional anyway.

    Imo decimals work a lot better with some modernist themes, perhaps foremost spyked's.

    > Just a thought, I'd personally like to see custom syntax highlighting at some point for non-vpatch snippets.

    This is actually a very strong point. Getting auto-highlights for a few of the most used langs would be a pretty massive win in all directions, including removing the very clunky documentation tools currently in use. I dunno I want doxygen or whatever anymore, if there's native highlighting, adnotation, trackbacks, holy god perfect manual software.

    Would [1[ blabla ]] syntax be terrible ? Where 1 is an index value of supported langs ?

    > Bash definitely uses double-brackets as builtin operators.

    There'd be a number of extant Trilema articles that'd be problematic atm (1, 2, plus a dozen or so log days, such as say randomly 3). The problem's not merely bash, but also plain regexp, xml data formats, etc.

    > Yup, exactly it.

    You know, the fact you added something so the ]] isn't matched anymore doesn't mean [[ isn't still.

  4. billymg says:

    > Imo decimals work a lot better with some modernist themes, perhaps foremost spyked's

    To clarify my thinking around changing the (trivially adjustable) default footnote identifier from decimal to lowercase-roman: every republican blog (including this one until making this update) used decimal, except Trilema (and I now notice Fixpoint uses roman numerals as well). My assumption was that blogs were all using decimal because no one bothered to change the default (perhaps I'm wrong). But Trilema (and Fixpoint) using something other than the default meant it must have been intentional. So I figured rather than make MP adjust it on his blog, just change the default that most probably didn't think twice about in the first place. And I kinda like the roman numerals now.

    That being said, it really makes no difference to me what that constant is set to, as it has no effect on the functioning of the code and can be easily changed by whoever is using it. It sounds like people prefer I leave it as it was, set to decimal by default?

    > Would [1[ blabla ]] syntax be terrible ? Where 1 is an index value of supported langs ?

    This certainly seems fine to me. Something for a future patch to build on imo.

    > There'd be a number of extant Trilema articles that'd be problematic atm (1, 2, plus a dozen or so log days, such as say randomly 3). The problem's not merely bash, but also plain regexp, xml data formats, etc.

    > You know, the fact you added something so the ]] isn't matched anymore doesn't mean [[ isn't still.

    The way the regex matches though is by the open and close delimiters. So there can be as many [[ in the code as you want and it won't trip up the matcher. That's why adding that little slash in /]] was so effective at making this safe for bash, regex, xml, etc. Those example Trilema logs you linked to should work fine as well.

    On the other hand, though I quite like [[ /]] for its brevity and ability to work with just about any code example I could find, I am not married to it and it's just a string so I certainly don't mind changing it to something else.

    EDIT: To expand on that last bit, MP's suggestion of [1[ ]] is equally concise, and 1 could represent vpatch diffs since it's the first implementation. I think I might like this more than [[ /]] now.

  5. > So there can be as many [[ in the code as you want and it won't trip up the matcher.

    I think this view is at best naive. What's the result of [[x]]y[[z/]] iyo ? x]]y[[z or just z ?

  6. billymg says:

    > What's the result of [[x]]y[[z/]] iyo ? x]]y[[z or just z ?

    The current implementation would result in x]]y[[z formatted inside a codeblock (I just tested to confirm this). If you wanted it to be just z inside the codeblock you could use &lsqb;[ to escape the opening tag.

    Have you ever run into this with (()) on Trilema, for something you wanted to display normally and not as a footnote?

    For example, before I prevented the footnotes processing from being applied to codeblocks the footnotes matcher would inevitably turn up a few false positives in the code snippet.

    In any case I do think [1[ ... /]] is the better option, but even so, if for some reason you had [1[x]]y[1[z/]] and you only wanted z in the codeblock you'd be stuck escaping the opening tag again.

    I'm not trying to argue strongly for any one tag, just saying that I'm finding it very difficult to come up with something that will work as intended 100% of the time.

    FWIW markdown uses ``` code goes here ``` for codeblocks. But I understand spyked uses markdown for his blog so it would at least be a non-starter there.

  7. spyked says:

    > But I understand spyked uses markdown for his blog so it would at least be a non-starter there.

    Thinking a bit about this, I don't think the choice of opening/closing strings should be made based on who uses what, simply because... well, at some point anyone might choose any particular string for some reason or another, and then what. On the other hand it's damn near impossible to escape in-bandism, so I think this should rather be a configurable knob, more precisely a PHP constant (or two of them?) that's set to whatever the user desires.

    For the record, The Tar Pit uses the footnote functionality provided by the Markdown plugin instead of footnotes.php (which is also why the lack of footnote tooltips), so this wouldn't affect me either way. Moreover, IMHO it's on me to port the functionality from one place to the other if I need it, so I guess this solves that.

Leave a Reply