diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php index a2003c0aa4..78924963d6 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/functions.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/functions.php @@ -497,6 +497,37 @@ function trac_comment_code_block( $fence ) { $m['indent'] . '}}}'; } +/** + * Converts the ATX headings of one span to Trac headings. + * + * @param string $text A span of the pull request lying outside any fenced code block. + * @return string|false The span with its headings converted, or false if it cannot be built. + */ +function trac_comment_headings( $text ) { + $skipped = trac_comment_skipped(); + + // A heading is separated by whitespace alone; a `>` after its `=` run is text. + $space = str_replace( '>', '', $skipped ); + + $text = preg_replace( "~^{$skipped}*\K(?=={1,6}{$space})~mu", '!', $text ); + if ( null === $text ) { + return false; + } + + $text = preg_replace_callback( + '~^(?P[ \t]*>[ \t>]*)?[ \t]{0,3}(?P\#{1,6})[ \t]+(?P.+?)(?:[ \t]+\#+)?[ \t]*$~m', + function ( $m ) { + $level = str_repeat( '=', strlen( $m['hashes'] ) ); + $cite = '' === $m['cite'] ? '' : rtrim( $m['cite'] ) . ' '; + + return "{$cite}{$level} {$m['text']} {$level}"; + }, + $text + ); + + return is_string( $text ) ? $text : false; +} + /** * Converts one span of pull request text to Trac wiki markup. * @@ -642,6 +673,7 @@ function ( $m ) { * This: * - Strips HTML comments * - Converts code blocks + * - Converts headings * - Converts image embeds * - Converts links * - Converts tables @@ -666,7 +698,14 @@ function format_github_content_for_trac_comment( $desc ) { } foreach ( $parts as $i => $part ) { - $part = ( $i % 2 ) ? trac_comment_code_block( $part ) : trac_comment_wiki_text( $part ); + if ( $i % 2 ) { + $part = trac_comment_code_block( $part ); + } else { + // A heading is a whole line, and only this split's spans begin and end where lines do. + $part = trac_comment_headings( $part ); + $part = is_string( $part ) ? trac_comment_wiki_text( $part ) : false; + } + if ( false === $part ) { return false; } diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php new file mode 100644 index 0000000000..1b781653d6 --- /dev/null +++ b/api.wordpress.org/public_html/dotorg/trac/pr/tests/Trac_Comment_Heading_Test.php @@ -0,0 +1,314 @@ +assertIsString( $desc ); + $this->assertStringContainsString( $expect, $desc ); + } + + /** + * Supplies one pull request heading per level. + * + * @return array Bodies and the headings they must compose. + */ + public function data_heading_levels(): array { + return array( + 'level one' => array( "# Summary\n\nText.\n", '= Summary =' ), + 'level two' => array( "## Summary\n\nText.\n", '== Summary ==' ), + 'level three' => array( "### Summary\n\nText.\n", '=== Summary ===' ), + 'level four' => array( "#### Summary\n\nText.\n", '==== Summary ====' ), + 'level five' => array( "##### Summary\n\nText.\n", '===== Summary =====' ), + 'level six' => array( "###### Summary\n\nText.\n", '====== Summary ======' ), + ); + } + + /** + * A seventh `#` is past Trac's deepest heading, so the line is left as text. + * + * @return void + */ + public function test_a_seventh_level_is_not_a_heading() { + $desc = format_github_content_for_trac_comment( "####### Summary\n\nText.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '####### Summary', $desc ); + } + + /** + * Markdown's optional closing run belongs to the marker, not to the title. + * + * @return void + */ + public function test_a_closed_heading_keeps_only_its_title() { + $desc = format_github_content_for_trac_comment( "## Summary ##\n\nText.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '== Summary ==', $desc ); + $this->assertStringNotContainsString( '## ', $desc ); + } + + /** + * Up to three leading spaces are Markdown's, and Trac has no use for them. + * + * @return void + */ + public function test_an_indented_heading_is_converted() { + $desc = format_github_content_for_trac_comment( "Text.\n\n ## Summary\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( "\n== Summary ==", $desc ); + } + + /** + * A line that only looks like a heading must reach Trac as it was written. + * + * @dataProvider data_non_headings + * + * @param string $body A pull request body opening a line with `#`. + * @return void + */ + public function test_a_hash_that_is_not_a_heading_is_left_alone( string $body ) { + $this->assertSame( trim( $body ), format_github_content_for_trac_comment( $body ) ); + } + + /** + * Supplies the line-opening hashes that are not Markdown headings. + * + * @return array Bodies that must be composed unchanged. + */ + public function data_non_headings(): array { + return array( + 'ticket reference' => array( "#65845 is the ticket.\n" ), + 'no space' => array( "#Summary\n" ), + 'interpreter line' => array( "#!/bin/sh\n" ), + 'empty heading' => array( "## \n" ), + 'four spaces' => array( "Text.\n\n ## Summary\n" ), + ); + } + + /** + * Trac strips a citation's `>` before it reads the line, so a quote has headings too. + * + * @dataProvider data_cited_headings + * + * @param string $body A pull request body quoting a heading. + * @param string $expect The quoted line it must compose. + * @return void + */ + public function test_a_cited_heading_keeps_its_citation( string $body, string $expect ) { + $desc = format_github_content_for_trac_comment( $body ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( $expect, $desc ); + } + + /** + * Supplies the quoted lines Trac reads as a heading. + * + * @return array Bodies and the quoted lines they must compose. + */ + public function data_cited_headings(): array { + return array( + 'converted' => array( "> ## Summary\n", '> == Summary ==' ), + 'no space' => array( ">## Summary\n", '> == Summary ==' ), + 'nested' => array( "> > ## Summary\n", '> > == Summary ==' ), + 'escaped' => array( "> == Summary\n", '> !== Summary' ), + 'escaped nested' => array( "> > == Summary\n", '> > !== Summary' ), + 'escaped indented' => array( "> == Summary\n", '> !== Summary' ), + ); + } + + /** + * A quotation deep enough to exhaust PCRE's stack must still reach the ticket. + * + * A pattern that repeats a group rather than naming a character class gives up on + * a line like this, and a refused span takes the whole comment with it. + * + * @dataProvider data_deep_citations + * + * @param string $line The line to quote, once the citation is prepended. + * @param string $expect The end of the line it must compose. + * @return void + */ + public function test_a_deep_citation_does_not_refuse_the_comment( string $line, string $expect ) { + // Two characters per level, within GitHub's 65536 character body. + $desc = format_github_content_for_trac_comment( str_repeat( '> ', 30000 ) . $line ); + + $this->assertIsString( $desc ); + $this->assertStringEndsWith( $expect, $desc ); + } + + /** + * Supplies the deeply quoted lines that must survive the composer. + * + * @return array Lines to quote and the endings they must compose. + */ + public function data_deep_citations(): array { + return array( + 'converted' => array( "## Summary\n", '== Summary ==' ), + 'escaped' => array( "== Summary\n", '!== Summary' ), + 'neither' => array( "Summary\n", '> Summary' ), + ); + } + + /** + * A heading's own inline code must stay inside the heading. + * + * The composer splits a span on its inline code, so a heading cut in half by a + * backtick would otherwise close over the first piece alone. + * + * @return void + */ + public function test_a_heading_holding_inline_code_stays_one_heading() { + $desc = format_github_content_for_trac_comment( "## The `retention-days` key\n\nText.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '== The `retention-days` key ==', $desc ); + } + + /** + * Inline code that opens a line is content, not a marker. + * + * @return void + */ + public function test_a_hash_inside_inline_code_is_not_a_heading() { + $desc = format_github_content_for_trac_comment( "Write `## Summary` for a heading.\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '`## Summary`', $desc ); + } + + /** + * A heading's links and images are converted with the rest of the prose. + * + * @return void + */ + public function test_a_heading_still_converts_its_links() { + $desc = format_github_content_for_trac_comment( "## See [the docs](https://example.org/doc)\n" ); + + $this->assertSame( '== See [https://example.org/doc the docs] ==', $desc ); + } + + /** + * A fenced block's contents are code, so its hashes are not headings. + * + * @return void + */ + public function test_a_hash_inside_a_fence_is_not_a_heading() { + $desc = format_github_content_for_trac_comment( "Text.\n\n```sh\n## Summary\n```\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( "\n## Summary\n", $desc ); + $this->assertStringNotContainsString( '== Summary ==', $desc ); + } + + /** + * Trac's own heading marker is the composer's to write, not the body's. + * + * @dataProvider data_wiki_headings + * + * @param string $body A pull request body opening a line with `=`. + * @param string $expect The escaped line it must compose. + * @return void + */ + public function test_a_wiki_heading_in_the_body_is_escaped( string $body, string $expect ) { + $desc = format_github_content_for_trac_comment( $body ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( $expect, $desc ); + } + + /** + * Supplies the line-opening `=` runs Trac would read as a heading. + * + * @return array Bodies and the escaped lines they must compose. + */ + public function data_wiki_headings(): array { + return array( + 'bare' => array( "== Summary\n", '!== Summary' ), + 'closed' => array( "== Summary ==\n", '!== Summary ==' ), + 'indented' => array( "Text.\n\n = Summary =\n", "\n !=" ), + 'deepest' => array( "====== Summary\n", '!====== Summary' ), + 'tab spaced' => array( "==\tSummary\n", "!==\tSummary" ), + // Trac steps over Python's whitespace, which is wider than PCRE's. + 'no-break' => array( "Text.\n\n\u{00A0}== Summary\n", "\u{00A0}!== Summary" ), + 'en space' => array( "Text.\n\n\u{2002}== Summary\n", "\u{2002}!== Summary" ), + 'ideographic' => array( "Text.\n\n\u{3000}== Summary\n", "\u{3000}!== Summary" ), + 'unit separator' => array( "Text.\n\n\x1f== Summary\n", "\x1f!== Summary" ), + // The same class again, this time separating the marker from the title. + 'no-break sep' => array( "Text.\n\n==\u{00A0}Summary\n", "!==\u{00A0}Summary" ), + 'en space sep' => array( "Text.\n\n==\u{2002}Summary\n", "!==\u{2002}Summary" ), + 'cited sep' => array( "Text.\n\n> ==\u{00A0}Summary\n", "> !==\u{00A0}Summary" ), + ); + } + + /** + * An `=` that opens no heading of Trac's is prose, and stays prose. + * + * @dataProvider data_inert_equals + * + * @param string $body A pull request body opening a line with `=`. + * @return void + */ + public function test_an_inert_equals_is_left_alone( string $body ) { + $this->assertSame( trim( $body ), format_github_content_for_trac_comment( $body ) ); + } + + /** + * Supplies the line-opening `=` runs Trac reads as text. + * + * @return array Bodies that must be composed unchanged. + */ + public function data_inert_equals(): array { + return array( + 'no space' => array( "=Summary\n" ), + 'citation' => array( "==> Not a heading.\n" ), + 'too deep' => array( "======= Summary\n" ), + 'arrow' => array( "=> Returns the value.\n" ), + ); + } + + /** + * A heading must not gain the escape written for the line above it. + * + * @return void + */ + public function test_an_escape_and_a_heading_can_share_a_body() { + $desc = format_github_content_for_trac_comment( "== Written\n\n## Composed\n" ); + + $this->assertIsString( $desc ); + $this->assertStringContainsString( '!== Written', $desc ); + $this->assertStringContainsString( '== Composed ==', $desc ); + $this->assertStringNotContainsString( '!== Composed', $desc ); + } +} diff --git a/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php b/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php index 7e705b9709..fe0da1fe8c 100644 --- a/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php +++ b/api.wordpress.org/public_html/dotorg/trac/pr/webhook.php @@ -73,7 +73,7 @@ function verify_signature() { foreach ( $existing_refs as $ref ) { if ( $ref->trac === $pr_data->trac_ticket[0] && - $ref->ticket === $pr_data->trac_ticket[1] + (int) $ref->ticket === (int) $pr_data->trac_ticket[1] ) { $matched_existing_ref = true; } @@ -85,11 +85,14 @@ function verify_signature() { unset( $_pr_data_no_ticket->trac_ticket, $_pr_data_no_ticket->body ); // Step 3. If not in DB, or $pr_data->trac_ticket isn't yet in the DB, add a new row of it. + $user_id = 0; + $new_ref = false; + if ( $pr_data->trac_ticket && ( ! $existing_refs || ! $matched_existing_ref ) ) { $user_id = (int) find_wporg_user_by_github( $pr_data->user->name, 'ID' ); - $wpdb->insert( + $new_ref = (bool) $wpdb->insert( 'trac_github_prs', [ 'created' => gmdate( 'Y-m-d H:i:s', strtotime( $pr_data->created_at ) ), @@ -102,6 +105,10 @@ function verify_signature() { 'author' => $user_id, ] ); + } + + // Only the request whose row was added mentions the PR on the ticket. + if ( $new_ref ) { // Add a mention to the Trac Ticket. $trac = get_trac_instance( $pr_data->trac_ticket[0] );