From fad98b87a0125ad175b8bdcf4df4dd548afd59e4 Mon Sep 17 00:00:00 2001 From: Daniel Naranjo Date: Wed, 25 Jun 2025 17:09:50 -0400 Subject: [PATCH 1/4] Add clickable hyperlinks to diff IDs in arc flow - Add PhutilConsoleFormatter class with formatHyperlink() method - Update ICFlowMonogramField to render clickable diff IDs (D123) - Uses OSC 8 terminal hyperlinks for supported terminals - Gracefully falls back to plain text in unsupported terminals - Configurable base URL via UBER_DIFFERENTIAL_BASE_URL environment variable - Defaults to https://code.uberinternal.com Users can now click on diff IDs in arc flow output to open them in browser. --- src/console/PhutilConsoleFormatter.php | 104 +++++++++++++++++++++++++ src/flow/field/ICFlowMonogramField.php | 50 +++++++++++- 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 src/console/PhutilConsoleFormatter.php diff --git a/src/console/PhutilConsoleFormatter.php b/src/console/PhutilConsoleFormatter.php new file mode 100644 index 000000000..809e2074a --- /dev/null +++ b/src/console/PhutilConsoleFormatter.php @@ -0,0 +1,104 @@ + 31, + 'green' => 32, + 'yellow' => 33, + 'blue' => 34, + 'magenta' => 35, + 'cyan' => 36, + 'white' => 37, + 'default' => 39, + ); + + /** + * Disable ANSI color and formatting codes. + */ + public static function disableANSI($disable) { + self::$disableANSI = $disable; + } + + /** + * Get the current ANSI disabled state. + */ + public static function getDisableANSI() { + return self::$disableANSI; + } + + /** + * Format a string with ANSI color codes. + */ + public static function formatString($format, $string = null) { + if (self::$disableANSI) { + return $string; + } + + $colors = self::$colorCodes; + $codes = array(); + + if (strpos($format, 'bold') !== false) { + $codes[] = 1; + } + + foreach ($colors as $color => $code) { + if (strpos($format, $color) !== false) { + $codes[] = $code; + } + } + + if (empty($codes)) { + return $string; + } + + $prefix = sprintf('\033[%sm', implode(';', $codes)); + $suffix = '\033[0m'; + + return $prefix.$string.$suffix; + } + + /** + * Strip ANSI escape sequences from a string. + */ + public static function stripANSI($string) { + return preg_replace('/\x1b\[[0-9;]*m/', '', $string); + } + + /** + * Escape format codes in a string. + */ + public static function escapeFormat($format) { + return addcslashes($format, '*_#'); + } + + /** + * Unescape format codes in a string. + */ + public static function unescapeFormat($format) { + return preg_replace('/\\\\(\*\*.*\*\*|__.*__|##.*##)/sU', '\1', $format); + } + + /** + * Create a terminal hyperlink using OSC 8 escape sequences. + * + * @param string $url The URL to link to + * @param string $text The visible text to display + * @return string Terminal hyperlink or just text if ANSI is disabled + */ + public static function formatHyperlink($url, $text) { + if (self::getDisableANSI()) { + // If ANSI is disabled, just return the text + return $text; + } + + // OSC 8 hyperlink format: \033]8;;URL\033\TEXT\033]8;;\033\ + $esc = chr(27); + return $esc.']8;;'.$url.$esc.'\\'.$text.$esc.']8;;'.$esc.'\\'; + } + +} diff --git a/src/flow/field/ICFlowMonogramField.php b/src/flow/field/ICFlowMonogramField.php index 3d31b8e12..01b9ee0f2 100644 --- a/src/flow/field/ICFlowMonogramField.php +++ b/src/flow/field/ICFlowMonogramField.php @@ -17,7 +17,18 @@ public function getDefaultFieldOrder() { } protected function renderValues(array $values) { - return 'D'.idx($values, 'revision-id'); + $revision_id = idx($values, 'revision-id'); + $monogram = 'D'.$revision_id; + + // Create clickable link if we have a revision ID + if ($revision_id) { + $url = $this->buildDifferentialURL($revision_id); + if ($url) { + return PhutilConsoleFormatter::formatHyperlink($url, $monogram); + } + } + + return $monogram; } public function getValues(ICFlowFeature $feature) { @@ -28,4 +39,41 @@ public function getValues(ICFlowFeature $feature) { return null; } + /** + * Build the URL for a differential revision. + * + * @param int $revision_id The revision ID (e.g., 123 for D123) + * @return string|null The full URL or null if no base URL configured + */ + private function buildDifferentialURL($revision_id) { + // Try to get the base URL from configuration + $base_url = $this->getDifferentialBaseURL(); + if (!$base_url) { + return null; + } + + // Ensure base URL ends with a slash + $base_url = rtrim($base_url, '/'); + + return $base_url.'/D'.$revision_id; + } + + /** + * Get the base URL for differential from configuration. + * + * @return string|null The base URL or null if not configured + */ + private function getDifferentialBaseURL() { + // Try multiple configuration sources in order of preference + + // 1. Check for Uber-specific configuration + $uber_url = getenv('UBER_DIFFERENTIAL_BASE_URL'); + if ($uber_url) { + return $uber_url; + } + + // 2. Default to Uber internal URL + return 'https://code.uberinternal.com'; + } + } From 34b0be20cd5be978005d6612a33d8136f1f48c4e Mon Sep 17 00:00:00 2001 From: Daniel Naranjo Date: Wed, 25 Jun 2025 17:31:00 -0400 Subject: [PATCH 2/4] Add comprehensive tests for clickable diff functionality - Add PhutilConsoleFormatterTestCase with tests for: - formatHyperlink() method with OSC 8 sequences - ANSI enable/disable functionality - Edge cases and format string methods - Text escaping/unescaping - Add ICFlowMonogramFieldTestCase with tests for: - Field key and configuration methods - Value rendering with and without hyperlinks - URL building and base URL configuration - Environment variable support - Edge cases and error conditions All tests pass and verify proper functionality of the clickable diff ID feature in arc flow output. --- .../PhutilConsoleFormatterTestCase.php | 141 ++++++++++++ .../__tests__/ICFlowMonogramFieldTestCase.php | 202 ++++++++++++++++++ 2 files changed, 343 insertions(+) create mode 100644 src/console/__tests__/PhutilConsoleFormatterTestCase.php create mode 100644 src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php diff --git a/src/console/__tests__/PhutilConsoleFormatterTestCase.php b/src/console/__tests__/PhutilConsoleFormatterTestCase.php new file mode 100644 index 000000000..1abd8e9c8 --- /dev/null +++ b/src/console/__tests__/PhutilConsoleFormatterTestCase.php @@ -0,0 +1,141 @@ +assertEqual( + $expected, + $result, + pht('formatHyperlink should produce OSC 8 escape sequences when ANSI is enabled')); + + // Test with ANSI disabled + PhutilConsoleFormatter::disableANSI(true); + $result = PhutilConsoleFormatter::formatHyperlink($url, $text); + + $this->assertEqual( + $text, + $result, + pht('formatHyperlink should return plain text when ANSI is disabled')); + + // Reset ANSI state + PhutilConsoleFormatter::disableANSI(false); + } + + public function testFormatHyperlinkEdgeCases() { + // Test empty URL + $result = PhutilConsoleFormatter::formatHyperlink('', 'text'); + $esc = chr(27); + $expected = $esc.']8;;'.$esc.'\\'.'text'.$esc.']8;;'.$esc.'\\'; + $this->assertEqual($expected, $result); + + // Test empty text + $result = PhutilConsoleFormatter::formatHyperlink('https://example.com', ''); + $esc = chr(27); + $expected = $esc.']8;;https://example.com'.$esc.'\\'.$esc.']8;;'.$esc.'\\'; + $this->assertEqual($expected, $result); + + // Test special characters in URL + $url = 'https://example.com/path?query=value&other=123#fragment'; + $text = 'Link Text'; + $result = PhutilConsoleFormatter::formatHyperlink($url, $text); + $esc = chr(27); + $expected = $esc.']8;;'.$url.$esc.'\\'.$text.$esc.']8;;'.$esc.'\\'; + $this->assertEqual($expected, $result); + } + + public function testDisableANSI() { + // Test initial state + $this->assertFalse( + PhutilConsoleFormatter::getDisableANSI(), + pht('ANSI should be enabled by default')); + + // Test disabling + PhutilConsoleFormatter::disableANSI(true); + $this->assertTrue( + PhutilConsoleFormatter::getDisableANSI(), + pht('ANSI should be disabled after calling disableANSI(true)')); + + // Test re-enabling + PhutilConsoleFormatter::disableANSI(false); + $this->assertFalse( + PhutilConsoleFormatter::getDisableANSI(), + pht('ANSI should be enabled after calling disableANSI(false)')); + } + + public function testFormatString() { + PhutilConsoleFormatter::disableANSI(false); + + // Test bold formatting + $result = PhutilConsoleFormatter::formatString('bold', 'test'); + $this->assertEqual("\033[1mtest\033[0m", $result); + + // Test color formatting + $result = PhutilConsoleFormatter::formatString('red', 'test'); + $this->assertEqual("\033[31mtest\033[0m", $result); + + // Test combined formatting + $result = PhutilConsoleFormatter::formatString('bold red', 'test'); + $this->assertEqual("\033[1;31mtest\033[0m", $result); + + // Test with ANSI disabled + PhutilConsoleFormatter::disableANSI(true); + $result = PhutilConsoleFormatter::formatString('bold red', 'test'); + $this->assertEqual('test', $result); + + // Reset ANSI state + PhutilConsoleFormatter::disableANSI(false); + } + + public function testStripANSI() { + $input = "\033[1;31mHello\033[0m \033[32mWorld\033[0m"; + $expected = "Hello World"; + $result = PhutilConsoleFormatter::stripANSI($input); + + $this->assertEqual( + $expected, + $result, + pht('stripANSI should remove all ANSI escape sequences')); + + // Test with hyperlink sequences + $esc = chr(27); + $hyperlink = $esc.']8;;https://example.com'.$esc.'\\text'.$esc.']8;;'.$esc.'\\'; + $result = PhutilConsoleFormatter::stripANSI($hyperlink); + + // stripANSI only removes color sequences, not OSC 8 + $this->assertEqual($hyperlink, $result); + } + + public function testEscapeFormat() { + $input = '**bold** __italic__ ##code##'; + $expected = '\\*\\*bold\\*\\* \\_\\_italic\\_\\_ \\#\\#code\\#\\#'; + $result = PhutilConsoleFormatter::escapeFormat($input); + + $this->assertEqual( + $expected, + $result, + pht('escapeFormat should escape format characters')); + } + + public function testUnescapeFormat() { + $input = '\\\\(**bold**) \\\\(__italic__) \\\\(##code##)'; + $expected = '(**bold**) (__italic__) (##code##)'; + $result = PhutilConsoleFormatter::unescapeFormat($input); + + $this->assertEqual( + $expected, + $result, + pht('unescapeFormat should unescape format sequences')); + } + +} diff --git a/src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php b/src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php new file mode 100644 index 000000000..c6d756de2 --- /dev/null +++ b/src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php @@ -0,0 +1,202 @@ +getMockBuilder('ICFlowFeature') + ->setMethods(array('getRevisionID')) + ->getMock(); + + $feature->method('getRevisionID') + ->willReturn($revision_id); + + return $feature; + } + + public function testGetFieldKey() { + $field = new ICFlowMonogramField(); + $this->assertEqual('monogram', $field->getFieldKey()); + } + + public function testGetValues() { + $field = new ICFlowMonogramField(); + + // Test with valid revision ID + $feature = $this->createMockFeature(12345); + $values = $field->getValues($feature); + $expected = array('revision-id' => 12345); + $this->assertEqual($expected, $values); + + // Test with null revision ID + $feature = $this->createMockFeature(null); + $values = $field->getValues($feature); + $this->assertEqual(null, $values); + + // Test with zero revision ID + $feature = $this->createMockFeature(0); + $values = $field->getValues($feature); + $this->assertEqual(null, $values); + } + + public function testRenderValuesWithoutHyperlinks() { + $field = new ICFlowMonogramField(); + + // Test basic monogram rendering + $values = array('revision-id' => 12345); + + // Disable ANSI to test plain text fallback + PhutilConsoleFormatter::disableANSI(true); + $result = $field->renderValues($values); + $this->assertEqual('D12345', $result); + + // Reset ANSI state + PhutilConsoleFormatter::disableANSI(false); + } + + public function testRenderValuesWithHyperlinks() { + $field = new ICFlowMonogramField(); + + // Test hyperlink rendering with ANSI enabled + $values = array('revision-id' => 12345); + + PhutilConsoleFormatter::disableANSI(false); + $result = $field->renderValues($values); + + // Should contain OSC 8 escape sequences + $this->assertTrue( + strpos($result, chr(27).']8;;') !== false, + pht('Result should contain OSC 8 hyperlink escape sequences')); + + // Should contain the expected URL + $this->assertTrue( + strpos($result, 'https://code.uberinternal.com/D12345') !== false, + pht('Result should contain the expected URL')); + + // Should contain the monogram text + $this->assertTrue( + strpos($result, 'D12345') !== false, + pht('Result should contain the monogram text')); + } + + public function testRenderValuesEdgeCases() { + $field = new ICFlowMonogramField(); + + // Test with empty values + $result = $field->renderValues(array()); + $this->assertEqual('D', $result); + + // Test with null revision-id + $values = array('revision-id' => null); + $result = $field->renderValues($values); + $this->assertEqual('D', $result); + + // Test with different revision IDs + $test_cases = array(1, 999, 123456789); + + foreach ($test_cases as $revision_id) { + $values = array('revision-id' => $revision_id); + $result = $field->renderValues($values); + + // With ANSI disabled, should return plain monogram + PhutilConsoleFormatter::disableANSI(true); + $plain_result = $field->renderValues($values); + $this->assertEqual("D{$revision_id}", $plain_result); + + // With ANSI enabled, should contain hyperlink + PhutilConsoleFormatter::disableANSI(false); + $hyperlink_result = $field->renderValues($values); + $this->assertTrue( + strpos($hyperlink_result, "D{$revision_id}") !== false, + pht("Result should contain D{$revision_id}")); + } + + // Reset ANSI state + PhutilConsoleFormatter::disableANSI(false); + } + + public function testBuildDifferentialURL() { + $field = new ICFlowMonogramField(); + + // Use reflection to test private method + $reflection = new ReflectionClass($field); + $method = $reflection->getMethod('buildDifferentialURL'); + $method->setAccessible(true); + + // Test URL building + $result = $method->invoke($field, 12345); + $this->assertEqual( + 'https://code.uberinternal.com/D12345', + $result); + + // Test with different revision ID + $result = $method->invoke($field, 67890); + $this->assertEqual( + 'https://code.uberinternal.com/D67890', + $result); + } + + public function testGetDifferentialBaseURL() { + $field = new ICFlowMonogramField(); + + // Use reflection to test private method + $reflection = new ReflectionClass($field); + $method = $reflection->getMethod('getDifferentialBaseURL'); + $method->setAccessible(true); + + // Test default URL + $result = $method->invoke($field); + $this->assertEqual( + 'https://code.uberinternal.com', + $result); + } + + public function testGetDifferentialBaseURLWithEnvironmentVariable() { + $field = new ICFlowMonogramField(); + + // Use reflection to test private method + $reflection = new ReflectionClass($field); + $method = $reflection->getMethod('getDifferentialBaseURL'); + $method->setAccessible(true); + + // Test with environment variable + $original_env = getenv('UBER_DIFFERENTIAL_BASE_URL'); + putenv('UBER_DIFFERENTIAL_BASE_URL=https://custom.uber.internal'); + + $result = $method->invoke($field); + $this->assertEqual( + 'https://custom.uber.internal', + $result); + + // Restore original environment + if ($original_env !== false) { + putenv('UBER_DIFFERENTIAL_BASE_URL=' . $original_env); + } else { + putenv('UBER_DIFFERENTIAL_BASE_URL'); + } + } + + public function testGetSummary() { + $field = new ICFlowMonogramField(); + $summary = $field->getSummary(); + + $this->assertTrue( + is_string($summary) && strlen($summary) > 0, + pht('Summary should be a non-empty string')); + + $this->assertTrue( + strpos($summary, 'DNNN') !== false, + pht('Summary should mention the DNNN format')); + } + + public function testGetDefaultFieldOrder() { + $field = new ICFlowMonogramField(); + $order = $field->getDefaultFieldOrder(); + + $this->assertTrue( + is_int($order), + pht('Field order should be an integer')); + + $this->assertEqual(3, $order); + } + +} From 5c4b8f53e0817b01a00249dfbf40e14e0952b8ca Mon Sep 17 00:00:00 2001 From: Daniel Naranjo Date: Wed, 25 Jun 2025 17:35:07 -0400 Subject: [PATCH 3/4] Revert "Add comprehensive tests for clickable diff functionality" This reverts commit 34b0be20cd5be978005d6612a33d8136f1f48c4e. --- .../PhutilConsoleFormatterTestCase.php | 141 ------------ .../__tests__/ICFlowMonogramFieldTestCase.php | 202 ------------------ 2 files changed, 343 deletions(-) delete mode 100644 src/console/__tests__/PhutilConsoleFormatterTestCase.php delete mode 100644 src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php diff --git a/src/console/__tests__/PhutilConsoleFormatterTestCase.php b/src/console/__tests__/PhutilConsoleFormatterTestCase.php deleted file mode 100644 index 1abd8e9c8..000000000 --- a/src/console/__tests__/PhutilConsoleFormatterTestCase.php +++ /dev/null @@ -1,141 +0,0 @@ -assertEqual( - $expected, - $result, - pht('formatHyperlink should produce OSC 8 escape sequences when ANSI is enabled')); - - // Test with ANSI disabled - PhutilConsoleFormatter::disableANSI(true); - $result = PhutilConsoleFormatter::formatHyperlink($url, $text); - - $this->assertEqual( - $text, - $result, - pht('formatHyperlink should return plain text when ANSI is disabled')); - - // Reset ANSI state - PhutilConsoleFormatter::disableANSI(false); - } - - public function testFormatHyperlinkEdgeCases() { - // Test empty URL - $result = PhutilConsoleFormatter::formatHyperlink('', 'text'); - $esc = chr(27); - $expected = $esc.']8;;'.$esc.'\\'.'text'.$esc.']8;;'.$esc.'\\'; - $this->assertEqual($expected, $result); - - // Test empty text - $result = PhutilConsoleFormatter::formatHyperlink('https://example.com', ''); - $esc = chr(27); - $expected = $esc.']8;;https://example.com'.$esc.'\\'.$esc.']8;;'.$esc.'\\'; - $this->assertEqual($expected, $result); - - // Test special characters in URL - $url = 'https://example.com/path?query=value&other=123#fragment'; - $text = 'Link Text'; - $result = PhutilConsoleFormatter::formatHyperlink($url, $text); - $esc = chr(27); - $expected = $esc.']8;;'.$url.$esc.'\\'.$text.$esc.']8;;'.$esc.'\\'; - $this->assertEqual($expected, $result); - } - - public function testDisableANSI() { - // Test initial state - $this->assertFalse( - PhutilConsoleFormatter::getDisableANSI(), - pht('ANSI should be enabled by default')); - - // Test disabling - PhutilConsoleFormatter::disableANSI(true); - $this->assertTrue( - PhutilConsoleFormatter::getDisableANSI(), - pht('ANSI should be disabled after calling disableANSI(true)')); - - // Test re-enabling - PhutilConsoleFormatter::disableANSI(false); - $this->assertFalse( - PhutilConsoleFormatter::getDisableANSI(), - pht('ANSI should be enabled after calling disableANSI(false)')); - } - - public function testFormatString() { - PhutilConsoleFormatter::disableANSI(false); - - // Test bold formatting - $result = PhutilConsoleFormatter::formatString('bold', 'test'); - $this->assertEqual("\033[1mtest\033[0m", $result); - - // Test color formatting - $result = PhutilConsoleFormatter::formatString('red', 'test'); - $this->assertEqual("\033[31mtest\033[0m", $result); - - // Test combined formatting - $result = PhutilConsoleFormatter::formatString('bold red', 'test'); - $this->assertEqual("\033[1;31mtest\033[0m", $result); - - // Test with ANSI disabled - PhutilConsoleFormatter::disableANSI(true); - $result = PhutilConsoleFormatter::formatString('bold red', 'test'); - $this->assertEqual('test', $result); - - // Reset ANSI state - PhutilConsoleFormatter::disableANSI(false); - } - - public function testStripANSI() { - $input = "\033[1;31mHello\033[0m \033[32mWorld\033[0m"; - $expected = "Hello World"; - $result = PhutilConsoleFormatter::stripANSI($input); - - $this->assertEqual( - $expected, - $result, - pht('stripANSI should remove all ANSI escape sequences')); - - // Test with hyperlink sequences - $esc = chr(27); - $hyperlink = $esc.']8;;https://example.com'.$esc.'\\text'.$esc.']8;;'.$esc.'\\'; - $result = PhutilConsoleFormatter::stripANSI($hyperlink); - - // stripANSI only removes color sequences, not OSC 8 - $this->assertEqual($hyperlink, $result); - } - - public function testEscapeFormat() { - $input = '**bold** __italic__ ##code##'; - $expected = '\\*\\*bold\\*\\* \\_\\_italic\\_\\_ \\#\\#code\\#\\#'; - $result = PhutilConsoleFormatter::escapeFormat($input); - - $this->assertEqual( - $expected, - $result, - pht('escapeFormat should escape format characters')); - } - - public function testUnescapeFormat() { - $input = '\\\\(**bold**) \\\\(__italic__) \\\\(##code##)'; - $expected = '(**bold**) (__italic__) (##code##)'; - $result = PhutilConsoleFormatter::unescapeFormat($input); - - $this->assertEqual( - $expected, - $result, - pht('unescapeFormat should unescape format sequences')); - } - -} diff --git a/src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php b/src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php deleted file mode 100644 index c6d756de2..000000000 --- a/src/flow/field/__tests__/ICFlowMonogramFieldTestCase.php +++ /dev/null @@ -1,202 +0,0 @@ -getMockBuilder('ICFlowFeature') - ->setMethods(array('getRevisionID')) - ->getMock(); - - $feature->method('getRevisionID') - ->willReturn($revision_id); - - return $feature; - } - - public function testGetFieldKey() { - $field = new ICFlowMonogramField(); - $this->assertEqual('monogram', $field->getFieldKey()); - } - - public function testGetValues() { - $field = new ICFlowMonogramField(); - - // Test with valid revision ID - $feature = $this->createMockFeature(12345); - $values = $field->getValues($feature); - $expected = array('revision-id' => 12345); - $this->assertEqual($expected, $values); - - // Test with null revision ID - $feature = $this->createMockFeature(null); - $values = $field->getValues($feature); - $this->assertEqual(null, $values); - - // Test with zero revision ID - $feature = $this->createMockFeature(0); - $values = $field->getValues($feature); - $this->assertEqual(null, $values); - } - - public function testRenderValuesWithoutHyperlinks() { - $field = new ICFlowMonogramField(); - - // Test basic monogram rendering - $values = array('revision-id' => 12345); - - // Disable ANSI to test plain text fallback - PhutilConsoleFormatter::disableANSI(true); - $result = $field->renderValues($values); - $this->assertEqual('D12345', $result); - - // Reset ANSI state - PhutilConsoleFormatter::disableANSI(false); - } - - public function testRenderValuesWithHyperlinks() { - $field = new ICFlowMonogramField(); - - // Test hyperlink rendering with ANSI enabled - $values = array('revision-id' => 12345); - - PhutilConsoleFormatter::disableANSI(false); - $result = $field->renderValues($values); - - // Should contain OSC 8 escape sequences - $this->assertTrue( - strpos($result, chr(27).']8;;') !== false, - pht('Result should contain OSC 8 hyperlink escape sequences')); - - // Should contain the expected URL - $this->assertTrue( - strpos($result, 'https://code.uberinternal.com/D12345') !== false, - pht('Result should contain the expected URL')); - - // Should contain the monogram text - $this->assertTrue( - strpos($result, 'D12345') !== false, - pht('Result should contain the monogram text')); - } - - public function testRenderValuesEdgeCases() { - $field = new ICFlowMonogramField(); - - // Test with empty values - $result = $field->renderValues(array()); - $this->assertEqual('D', $result); - - // Test with null revision-id - $values = array('revision-id' => null); - $result = $field->renderValues($values); - $this->assertEqual('D', $result); - - // Test with different revision IDs - $test_cases = array(1, 999, 123456789); - - foreach ($test_cases as $revision_id) { - $values = array('revision-id' => $revision_id); - $result = $field->renderValues($values); - - // With ANSI disabled, should return plain monogram - PhutilConsoleFormatter::disableANSI(true); - $plain_result = $field->renderValues($values); - $this->assertEqual("D{$revision_id}", $plain_result); - - // With ANSI enabled, should contain hyperlink - PhutilConsoleFormatter::disableANSI(false); - $hyperlink_result = $field->renderValues($values); - $this->assertTrue( - strpos($hyperlink_result, "D{$revision_id}") !== false, - pht("Result should contain D{$revision_id}")); - } - - // Reset ANSI state - PhutilConsoleFormatter::disableANSI(false); - } - - public function testBuildDifferentialURL() { - $field = new ICFlowMonogramField(); - - // Use reflection to test private method - $reflection = new ReflectionClass($field); - $method = $reflection->getMethod('buildDifferentialURL'); - $method->setAccessible(true); - - // Test URL building - $result = $method->invoke($field, 12345); - $this->assertEqual( - 'https://code.uberinternal.com/D12345', - $result); - - // Test with different revision ID - $result = $method->invoke($field, 67890); - $this->assertEqual( - 'https://code.uberinternal.com/D67890', - $result); - } - - public function testGetDifferentialBaseURL() { - $field = new ICFlowMonogramField(); - - // Use reflection to test private method - $reflection = new ReflectionClass($field); - $method = $reflection->getMethod('getDifferentialBaseURL'); - $method->setAccessible(true); - - // Test default URL - $result = $method->invoke($field); - $this->assertEqual( - 'https://code.uberinternal.com', - $result); - } - - public function testGetDifferentialBaseURLWithEnvironmentVariable() { - $field = new ICFlowMonogramField(); - - // Use reflection to test private method - $reflection = new ReflectionClass($field); - $method = $reflection->getMethod('getDifferentialBaseURL'); - $method->setAccessible(true); - - // Test with environment variable - $original_env = getenv('UBER_DIFFERENTIAL_BASE_URL'); - putenv('UBER_DIFFERENTIAL_BASE_URL=https://custom.uber.internal'); - - $result = $method->invoke($field); - $this->assertEqual( - 'https://custom.uber.internal', - $result); - - // Restore original environment - if ($original_env !== false) { - putenv('UBER_DIFFERENTIAL_BASE_URL=' . $original_env); - } else { - putenv('UBER_DIFFERENTIAL_BASE_URL'); - } - } - - public function testGetSummary() { - $field = new ICFlowMonogramField(); - $summary = $field->getSummary(); - - $this->assertTrue( - is_string($summary) && strlen($summary) > 0, - pht('Summary should be a non-empty string')); - - $this->assertTrue( - strpos($summary, 'DNNN') !== false, - pht('Summary should mention the DNNN format')); - } - - public function testGetDefaultFieldOrder() { - $field = new ICFlowMonogramField(); - $order = $field->getDefaultFieldOrder(); - - $this->assertTrue( - is_int($order), - pht('Field order should be an integer')); - - $this->assertEqual(3, $order); - } - -} From be1f3f22f603d11d4911fe63cd336d408dccd367 Mon Sep 17 00:00:00 2001 From: Daniel Naranjo Date: Wed, 25 Jun 2025 17:35:17 -0400 Subject: [PATCH 4/4] Revert "Add clickable hyperlinks to diff IDs in arc flow" This reverts commit fad98b87a0125ad175b8bdcf4df4dd548afd59e4. --- src/console/PhutilConsoleFormatter.php | 104 ------------------------- src/flow/field/ICFlowMonogramField.php | 50 +----------- 2 files changed, 1 insertion(+), 153 deletions(-) delete mode 100644 src/console/PhutilConsoleFormatter.php diff --git a/src/console/PhutilConsoleFormatter.php b/src/console/PhutilConsoleFormatter.php deleted file mode 100644 index 809e2074a..000000000 --- a/src/console/PhutilConsoleFormatter.php +++ /dev/null @@ -1,104 +0,0 @@ - 31, - 'green' => 32, - 'yellow' => 33, - 'blue' => 34, - 'magenta' => 35, - 'cyan' => 36, - 'white' => 37, - 'default' => 39, - ); - - /** - * Disable ANSI color and formatting codes. - */ - public static function disableANSI($disable) { - self::$disableANSI = $disable; - } - - /** - * Get the current ANSI disabled state. - */ - public static function getDisableANSI() { - return self::$disableANSI; - } - - /** - * Format a string with ANSI color codes. - */ - public static function formatString($format, $string = null) { - if (self::$disableANSI) { - return $string; - } - - $colors = self::$colorCodes; - $codes = array(); - - if (strpos($format, 'bold') !== false) { - $codes[] = 1; - } - - foreach ($colors as $color => $code) { - if (strpos($format, $color) !== false) { - $codes[] = $code; - } - } - - if (empty($codes)) { - return $string; - } - - $prefix = sprintf('\033[%sm', implode(';', $codes)); - $suffix = '\033[0m'; - - return $prefix.$string.$suffix; - } - - /** - * Strip ANSI escape sequences from a string. - */ - public static function stripANSI($string) { - return preg_replace('/\x1b\[[0-9;]*m/', '', $string); - } - - /** - * Escape format codes in a string. - */ - public static function escapeFormat($format) { - return addcslashes($format, '*_#'); - } - - /** - * Unescape format codes in a string. - */ - public static function unescapeFormat($format) { - return preg_replace('/\\\\(\*\*.*\*\*|__.*__|##.*##)/sU', '\1', $format); - } - - /** - * Create a terminal hyperlink using OSC 8 escape sequences. - * - * @param string $url The URL to link to - * @param string $text The visible text to display - * @return string Terminal hyperlink or just text if ANSI is disabled - */ - public static function formatHyperlink($url, $text) { - if (self::getDisableANSI()) { - // If ANSI is disabled, just return the text - return $text; - } - - // OSC 8 hyperlink format: \033]8;;URL\033\TEXT\033]8;;\033\ - $esc = chr(27); - return $esc.']8;;'.$url.$esc.'\\'.$text.$esc.']8;;'.$esc.'\\'; - } - -} diff --git a/src/flow/field/ICFlowMonogramField.php b/src/flow/field/ICFlowMonogramField.php index 01b9ee0f2..3d31b8e12 100644 --- a/src/flow/field/ICFlowMonogramField.php +++ b/src/flow/field/ICFlowMonogramField.php @@ -17,18 +17,7 @@ public function getDefaultFieldOrder() { } protected function renderValues(array $values) { - $revision_id = idx($values, 'revision-id'); - $monogram = 'D'.$revision_id; - - // Create clickable link if we have a revision ID - if ($revision_id) { - $url = $this->buildDifferentialURL($revision_id); - if ($url) { - return PhutilConsoleFormatter::formatHyperlink($url, $monogram); - } - } - - return $monogram; + return 'D'.idx($values, 'revision-id'); } public function getValues(ICFlowFeature $feature) { @@ -39,41 +28,4 @@ public function getValues(ICFlowFeature $feature) { return null; } - /** - * Build the URL for a differential revision. - * - * @param int $revision_id The revision ID (e.g., 123 for D123) - * @return string|null The full URL or null if no base URL configured - */ - private function buildDifferentialURL($revision_id) { - // Try to get the base URL from configuration - $base_url = $this->getDifferentialBaseURL(); - if (!$base_url) { - return null; - } - - // Ensure base URL ends with a slash - $base_url = rtrim($base_url, '/'); - - return $base_url.'/D'.$revision_id; - } - - /** - * Get the base URL for differential from configuration. - * - * @return string|null The base URL or null if not configured - */ - private function getDifferentialBaseURL() { - // Try multiple configuration sources in order of preference - - // 1. Check for Uber-specific configuration - $uber_url = getenv('UBER_DIFFERENTIAL_BASE_URL'); - if ($uber_url) { - return $uber_url; - } - - // 2. Default to Uber internal URL - return 'https://code.uberinternal.com'; - } - }