Skip to content

Conversation

@charles-zablit
Copy link
Contributor

This reverts commit 64e1991.

#168603 breaks the CI because the bots support Unicode but the tests expect non Unicode characters.

@charles-zablit charles-zablit requested review from JDevlieghere and adrian-prantl and removed request for JDevlieghere December 4, 2025 18:04
@charles-zablit charles-zablit self-assigned this Dec 4, 2025
@llvmbot llvmbot added the lldb label Dec 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2025

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

Changes

This reverts commit 64e1991.

#168603 breaks the CI because the bots support Unicode but the tests expect non Unicode characters.


Full diff: https://github.com/llvm/llvm-project/pull/170711.diff

3 Files Affected:

  • (modified) lldb/include/lldb/Host/Terminal.h (-12)
  • (modified) lldb/source/Host/common/DiagnosticsRendering.cpp (+5-3)
  • (modified) lldb/source/Host/common/Terminal.cpp (-15)
diff --git a/lldb/include/lldb/Host/Terminal.h b/lldb/include/lldb/Host/Terminal.h
index 3d66515c18812..da0d05e8bd265 100644
--- a/lldb/include/lldb/Host/Terminal.h
+++ b/lldb/include/lldb/Host/Terminal.h
@@ -68,18 +68,6 @@ class Terminal {
 
   llvm::Error SetHardwareFlowControl(bool enabled);
 
-  /// Returns whether or not the current terminal supports Unicode rendering.
-  ///
-  /// The value is cached after the first computation.
-  ///
-  /// On POSIX systems, we check if the LANG environment variable contains the
-  /// substring "UTF-8", case insensitive.
-  ///
-  /// On Windows, we always return true since we use the `WriteConsoleW` API
-  /// internally. Note that the default Windows codepage (437) does not support
-  /// all Unicode characters. This function does not check the codepage.
-  static bool SupportsUnicode();
-
 protected:
   struct Data;
 
diff --git a/lldb/source/Host/common/DiagnosticsRendering.cpp b/lldb/source/Host/common/DiagnosticsRendering.cpp
index 375d1caa99f52..f2cd3968967fb 100644
--- a/lldb/source/Host/common/DiagnosticsRendering.cpp
+++ b/lldb/source/Host/common/DiagnosticsRendering.cpp
@@ -7,8 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/common/DiagnosticsRendering.h"
-#include "lldb/Host/Terminal.h"
-
 #include <cstdint>
 
 using namespace lldb_private;
@@ -99,8 +97,12 @@ void RenderDiagnosticDetails(Stream &stream,
     return;
   }
 
+  // Since there is no other way to find this out, use the color
+  // attribute as a proxy for whether the terminal supports Unicode
+  // characters.  In the future it might make sense to move this into
+  // Host so it can be customized for a specific platform.
   llvm::StringRef cursor, underline, vbar, joint, hbar, spacer;
-  if (Terminal::SupportsUnicode()) {
+  if (stream.AsRawOstream().colors_enabled()) {
     cursor = "˄";
     underline = "˜";
     vbar = "│";
diff --git a/lldb/source/Host/common/Terminal.cpp b/lldb/source/Host/common/Terminal.cpp
index dd1dc75133f45..436dfd8130d9b 100644
--- a/lldb/source/Host/common/Terminal.cpp
+++ b/lldb/source/Host/common/Terminal.cpp
@@ -400,21 +400,6 @@ llvm::Error Terminal::SetHardwareFlowControl(bool enabled) {
 #endif // LLDB_ENABLE_TERMIOS
 }
 
-bool Terminal::SupportsUnicode() {
-  static std::optional<bool> result;
-  if (result)
-    return result.value();
-#ifdef _WIN32
-  return true;
-#else
-  const char *lang_var = std::getenv("LANG");
-  if (!lang_var)
-    return false;
-  result = llvm::StringRef(lang_var).lower().find("utf-8") != std::string::npos;
-#endif
-  return result.value();
-}
-
 TerminalState::TerminalState(Terminal term, bool save_process_group)
     : m_tty(term) {
   Save(term, save_process_group);

Copy link
Member

@dzhidzhoev dzhidzhoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(AFAIK approval is not necessary for (self-)reverting commits, though skip-precommit-approval tag is required)

@charles-zablit charles-zablit merged commit 95f6b3e into llvm:main Dec 4, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants