mirror of
https://github.com/phranck/TUIkit.git
synced 2026-05-21 09:50:35 +00:00
refactor: Phase 1 code quality improvements and ANSI cleanup
- Remove dead code: legacy BorderModifier (70 lines), legacy FrameModifier (74 lines) - Fix state-mismatch bug in Theme/Appearance setTheme()/setAppearance() - Eliminate force unwraps in Panel.body and Menu.dividerLineIndex - Remove anti-patterns: _ = self, pointless ternary, identical branches - Rename all 1-2 char variables to self-descriptive names across 9 files - Centralize ANSI constants: add dim, ansiPattern to ANSIRenderer - Replace all hardcoded ANSI reset strings with ANSIRenderer.reset - Replace raw ANSI regex with ANSIRenderer.ansiPattern in FrameBuffer - Inline all local ANSIRenderer alias variables (13 removals) - Replace manual CSI-building in BackgroundModifier with ANSIRenderer.backgroundCode() - Migrate View.frame() to use FlexibleFrameView - Update macOS deployment target from 10.15 to 13 - Remove redundant explicit raw values from DemoPage enum 22 files changed, 113 insertions(+), 310 deletions(-)
This commit is contained in:
phranck
+1
-1
@@ -8,7 +8,7 @@ let package = Package(
|
||||
// Minimum deployment targets for Apple platforms
|
||||
// Linux is automatically supported (no platform specification needed)
|
||||
platforms: [
|
||||
.macOS(.v10_15)
|
||||
.macOS(.v13)
|
||||
],
|
||||
products: [
|
||||
.library(
|
||||
|
||||
@@ -448,9 +448,8 @@ internal final class AppRunner<A: App> {
|
||||
EnvironmentStorage.shared.environment = environment
|
||||
|
||||
// Register for state changes
|
||||
AppState.shared.observe { [weak self] in
|
||||
AppState.shared.observe {
|
||||
needsRerender = true
|
||||
_ = self // Silence warning
|
||||
}
|
||||
|
||||
isRunning = true
|
||||
@@ -595,19 +594,19 @@ internal final class AppRunner<A: App> {
|
||||
|
||||
// Default handling (only if no handler consumed the event)
|
||||
switch event.key {
|
||||
case .character(let char) where char == "q" || char == "Q":
|
||||
case .character(let character) where character == "q" || character == "Q":
|
||||
// 'q' is the only way to quit (respects quitBehavior setting)
|
||||
if statusBar.isQuitAllowed {
|
||||
isRunning = false
|
||||
}
|
||||
|
||||
case .character(let char) where char == "t" || char == "T":
|
||||
case .character(let character) where character == "t" || character == "T":
|
||||
// 't' cycles theme (if theme item is enabled)
|
||||
if statusBar.showThemeItem {
|
||||
themeManager.cycleTheme()
|
||||
}
|
||||
|
||||
case .character(let char) where char == "a" || char == "A":
|
||||
case .character(let character) where character == "a" || character == "A":
|
||||
// 'a' cycles appearance
|
||||
appearanceManager.cycleAppearance()
|
||||
|
||||
|
||||
@@ -308,15 +308,11 @@ public final class AppearanceManager: @unchecked Sendable {
|
||||
public func setAppearance(_ appearance: Appearance) {
|
||||
if let index = availableAppearances.firstIndex(where: { $0.id == appearance.id }) {
|
||||
currentIndex = index
|
||||
} else {
|
||||
currentIndex = 0
|
||||
}
|
||||
|
||||
// Apply the appearance directly (even if not in availableAppearances)
|
||||
var environment = EnvironmentStorage.shared.environment
|
||||
environment.appearance = appearance
|
||||
EnvironmentStorage.shared.environment = environment
|
||||
AppState.shared.setNeedsRender()
|
||||
// If appearance is not in availableAppearances, currentIndex stays unchanged.
|
||||
// Only apply appearances that are in the available list to keep
|
||||
// currentAppearance and environment in sync.
|
||||
applyCurrentAppearance()
|
||||
}
|
||||
|
||||
/// Applies the current appearance to the environment and triggers a re-render.
|
||||
|
||||
@@ -179,32 +179,34 @@ public struct Color: Sendable, Equatable {
|
||||
/// - lightness: The lightness component (0-100).
|
||||
/// - Returns: The corresponding RGB color.
|
||||
public static func hsl(_ hue: Double, _ saturation: Double, _ lightness: Double) -> Color {
|
||||
let h = hue / 360.0
|
||||
let s = saturation / 100.0
|
||||
let l = lightness / 100.0
|
||||
let normalizedHue = hue / 360.0
|
||||
let normalizedSaturation = saturation / 100.0
|
||||
let normalizedLightness = lightness / 100.0
|
||||
|
||||
if s == 0 {
|
||||
if normalizedSaturation == 0 {
|
||||
// Achromatic (gray)
|
||||
let gray = UInt8(l * 255)
|
||||
let gray = UInt8(normalizedLightness * 255)
|
||||
return .rgb(gray, gray, gray)
|
||||
}
|
||||
|
||||
let q = l < 0.5 ? l * (1 + s) : l + s - l * s
|
||||
let p = 2 * l - q
|
||||
let chromaFactor = normalizedLightness < 0.5
|
||||
? normalizedLightness * (1 + normalizedSaturation)
|
||||
: normalizedLightness + normalizedSaturation - normalizedLightness * normalizedSaturation
|
||||
let luminanceFactor = 2 * normalizedLightness - chromaFactor
|
||||
|
||||
func hueToRGB(_ p: Double, _ q: Double, _ t: Double) -> Double {
|
||||
var t = t
|
||||
if t < 0 { t += 1 }
|
||||
if t > 1 { t -= 1 }
|
||||
if t < 1/6 { return p + (q - p) * 6 * t }
|
||||
if t < 1/2 { return q }
|
||||
if t < 2/3 { return p + (q - p) * (2/3 - t) * 6 }
|
||||
return p
|
||||
func hueToRGB(_ luminance: Double, _ chroma: Double, _ hueComponent: Double) -> Double {
|
||||
var adjustedHue = hueComponent
|
||||
if adjustedHue < 0 { adjustedHue += 1 }
|
||||
if adjustedHue > 1 { adjustedHue -= 1 }
|
||||
if adjustedHue < 1/6 { return luminance + (chroma - luminance) * 6 * adjustedHue }
|
||||
if adjustedHue < 1/2 { return chroma }
|
||||
if adjustedHue < 2/3 { return luminance + (chroma - luminance) * (2/3 - adjustedHue) * 6 }
|
||||
return luminance
|
||||
}
|
||||
|
||||
let red = UInt8(hueToRGB(p, q, h + 1/3) * 255)
|
||||
let green = UInt8(hueToRGB(p, q, h) * 255)
|
||||
let blue = UInt8(hueToRGB(p, q, h - 1/3) * 255)
|
||||
let red = UInt8(hueToRGB(luminanceFactor, chromaFactor, normalizedHue + 1/3) * 255)
|
||||
let green = UInt8(hueToRGB(luminanceFactor, chromaFactor, normalizedHue) * 255)
|
||||
let blue = UInt8(hueToRGB(luminanceFactor, chromaFactor, normalizedHue - 1/3) * 255)
|
||||
|
||||
return .rgb(red, green, blue)
|
||||
}
|
||||
|
||||
@@ -202,8 +202,8 @@ public final class FocusManager: @unchecked Sendable {
|
||||
|
||||
/// The currently focused element, if any.
|
||||
public var currentFocused: Focusable? {
|
||||
guard let id = focusedID else { return nil }
|
||||
return focusables.first { $0.focusID == id }
|
||||
guard let focusedIdentifier = focusedID else { return nil }
|
||||
return focusables.first { $0.focusID == focusedIdentifier }
|
||||
}
|
||||
|
||||
/// The ID of the currently focused element, if any.
|
||||
|
||||
@@ -667,16 +667,11 @@ public final class ThemeManager: @unchecked Sendable {
|
||||
public func setTheme(_ theme: Theme) {
|
||||
if let index = availableThemes.firstIndex(where: { $0.id == theme.id }) {
|
||||
currentIndex = index
|
||||
} else {
|
||||
// Theme not in list, add temporarily at current position
|
||||
currentIndex = 0
|
||||
}
|
||||
|
||||
// Apply the theme directly (even if not in availableThemes)
|
||||
var environment = EnvironmentStorage.shared.environment
|
||||
environment.theme = theme
|
||||
EnvironmentStorage.shared.environment = environment
|
||||
AppState.shared.setNeedsRender()
|
||||
// If theme is not in availableThemes, currentIndex stays unchanged.
|
||||
// Only apply themes that are in the available list to keep
|
||||
// currentTheme and environment in sync.
|
||||
applyCurrentTheme()
|
||||
}
|
||||
|
||||
/// Applies the current theme to the environment and triggers a re-render.
|
||||
|
||||
@@ -35,23 +35,7 @@ public struct BackgroundModifier: ViewModifier {
|
||||
|
||||
/// Applies background color to a string, preserving existing formatting.
|
||||
private func applyBackground(to string: String, color: Color) -> String {
|
||||
// Build the background escape sequence
|
||||
let bgCodes: [String]
|
||||
switch color.value {
|
||||
case .standard(let ansi):
|
||||
bgCodes = ["\(ansi.backgroundCode)"]
|
||||
case .bright(let ansi):
|
||||
bgCodes = ["\(ansi.brightBackgroundCode)"]
|
||||
case .palette256(let index):
|
||||
bgCodes = ["48", "5", "\(index)"]
|
||||
case .rgb(let red, let green, let blue):
|
||||
bgCodes = ["48", "2", "\(red)", "\(green)", "\(blue)"]
|
||||
}
|
||||
|
||||
let bgStart = "\u{1B}[\(bgCodes.joined(separator: ";"))m"
|
||||
let reset = ANSIRenderer.reset
|
||||
|
||||
return bgStart + string + reset
|
||||
ANSIRenderer.backgroundCode(for: color) + string + ANSIRenderer.reset
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -71,7 +71,6 @@ extension BorderedView: Renderable {
|
||||
|
||||
// Build the result
|
||||
var lines: [String] = []
|
||||
let reset = "\u{1B}[0m"
|
||||
|
||||
// Top border
|
||||
lines.append(colorize(topLine))
|
||||
@@ -81,7 +80,7 @@ extension BorderedView: Renderable {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let borderedLine = colorize(String(style.vertical))
|
||||
+ paddedLine
|
||||
+ reset
|
||||
+ ANSIRenderer.reset
|
||||
+ colorize(String(style.vertical))
|
||||
lines.append(borderedLine)
|
||||
}
|
||||
@@ -102,7 +101,6 @@ extension BorderedView: Renderable {
|
||||
/// ```
|
||||
private func renderBlockStyle(buffer: FrameBuffer, innerWidth: Int) -> FrameBuffer {
|
||||
var lines: [String] = []
|
||||
let reset = "\u{1B}[0m"
|
||||
|
||||
// For block style, use container background color for borders
|
||||
let containerBg = Color.theme.containerBackground
|
||||
@@ -116,7 +114,7 @@ extension BorderedView: Renderable {
|
||||
for line in buffer.lines {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let styledContent = applyBackground(paddedLine, background: containerBg)
|
||||
lines.append(sideBorder + styledContent + reset + sideBorder)
|
||||
lines.append(sideBorder + styledContent + ANSIRenderer.reset + sideBorder)
|
||||
}
|
||||
|
||||
// Bottom border: ▀▀▀ with FG = container BG
|
||||
@@ -136,8 +134,7 @@ extension BorderedView: Renderable {
|
||||
/// Applies a background color to content, re-applying after any resets.
|
||||
private func applyBackground(_ string: String, background: Color) -> String {
|
||||
let bgCode = ANSIRenderer.backgroundCode(for: background)
|
||||
let resetCode = "\u{1B}[0m"
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: resetCode, with: resetCode + bgCode)
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: ANSIRenderer.reset, with: ANSIRenderer.reset + bgCode)
|
||||
return bgCode + stringWithPersistentBg
|
||||
}
|
||||
|
||||
@@ -192,79 +189,4 @@ extension View {
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Legacy ViewModifier (kept for compatibility)
|
||||
|
||||
/// A modifier that adds a border around a view.
|
||||
///
|
||||
/// Note: This is the legacy implementation. The new `BorderedView`
|
||||
/// correctly handles available width constraints.
|
||||
public struct BorderModifier: ViewModifier {
|
||||
/// The border style to use.
|
||||
public let style: BorderStyle
|
||||
|
||||
/// The color of the border (nil uses default terminal color).
|
||||
public let color: Color?
|
||||
|
||||
public func modify(buffer: FrameBuffer, context: RenderContext) -> FrameBuffer {
|
||||
guard !buffer.isEmpty else { return buffer }
|
||||
|
||||
let contentWidth = buffer.width
|
||||
let innerWidth = max(contentWidth, 1)
|
||||
|
||||
// Build the top border line
|
||||
let topLine = buildBorderLine(
|
||||
left: style.topLeft,
|
||||
fill: style.horizontal,
|
||||
right: style.topRight,
|
||||
width: innerWidth
|
||||
)
|
||||
|
||||
// Build the bottom border line
|
||||
let bottomLine = buildBorderLine(
|
||||
left: style.bottomLeft,
|
||||
fill: style.horizontal,
|
||||
right: style.bottomRight,
|
||||
width: innerWidth
|
||||
)
|
||||
|
||||
// Build the result
|
||||
var lines: [String] = []
|
||||
|
||||
// Top border
|
||||
lines.append(colorize(topLine))
|
||||
|
||||
// Content lines with side borders
|
||||
// Important: Reset ANSI before right border to prevent color bleeding
|
||||
let reset = "\u{1B}[0m"
|
||||
for line in buffer.lines {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let borderedLine = colorize(String(style.vertical))
|
||||
+ paddedLine
|
||||
+ reset // Reset any styling from content
|
||||
+ colorize(String(style.vertical))
|
||||
lines.append(borderedLine)
|
||||
}
|
||||
|
||||
// Bottom border
|
||||
lines.append(colorize(bottomLine))
|
||||
|
||||
return FrameBuffer(lines: lines)
|
||||
}
|
||||
|
||||
/// Builds a horizontal border line.
|
||||
private func buildBorderLine(
|
||||
left: Character,
|
||||
fill: Character,
|
||||
right: Character,
|
||||
width: Int
|
||||
) -> String {
|
||||
String(left) + String(repeating: fill, count: width) + String(right)
|
||||
}
|
||||
|
||||
/// Applies color to a string, using theme border color as default.
|
||||
private func colorize(_ string: String) -> String {
|
||||
var textStyle = TextStyle()
|
||||
textStyle.foregroundColor = color ?? Color.theme.border
|
||||
return ANSIRenderer.render(string, with: textStyle)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,10 +46,6 @@ extension DimmedModifier: Renderable {
|
||||
private func applyDim(to text: String) -> String {
|
||||
guard !text.isEmpty else { return text }
|
||||
|
||||
// ANSI dim code
|
||||
let dimCode = "\u{1B}[2m"
|
||||
let resetCode = "\u{1B}[0m"
|
||||
|
||||
// If the line is empty (just spaces), keep it as is
|
||||
if text.stripped.trimmingCharacters(in: .whitespaces).isEmpty {
|
||||
return text
|
||||
@@ -58,7 +54,7 @@ extension DimmedModifier: Renderable {
|
||||
// Wrap the entire line in dim codes
|
||||
// Note: This adds dim at the start and reset at the end
|
||||
// Any existing styles will still work, but will be dimmed
|
||||
return dimCode + text + resetCode
|
||||
return ANSIRenderer.dim + text + ANSIRenderer.reset
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -44,8 +44,8 @@ extension FlexibleFrameView: Renderable {
|
||||
public func renderToBuffer(context: RenderContext) -> FrameBuffer {
|
||||
// Calculate the target width based on constraints
|
||||
let targetWidth: Int
|
||||
if let maxW = maxWidth {
|
||||
switch maxW {
|
||||
if let maximumWidth = maxWidth {
|
||||
switch maximumWidth {
|
||||
case .infinity:
|
||||
targetWidth = context.availableWidth
|
||||
case .fixed(let value):
|
||||
@@ -60,8 +60,8 @@ extension FlexibleFrameView: Renderable {
|
||||
|
||||
// Calculate the target height based on constraints
|
||||
let targetHeight: Int?
|
||||
if let maxH = maxHeight {
|
||||
switch maxH {
|
||||
if let maximumHeight = maxHeight {
|
||||
switch maximumHeight {
|
||||
case .infinity:
|
||||
targetHeight = context.availableHeight
|
||||
case .fixed(let value):
|
||||
@@ -87,18 +87,18 @@ extension FlexibleFrameView: Renderable {
|
||||
var finalWidth = buffer.width
|
||||
var finalHeight = buffer.height
|
||||
|
||||
if let minW = minWidth {
|
||||
finalWidth = max(finalWidth, minW)
|
||||
if let minimumWidth = minWidth {
|
||||
finalWidth = max(finalWidth, minimumWidth)
|
||||
}
|
||||
if let minH = minHeight {
|
||||
finalHeight = max(finalHeight, minH)
|
||||
if let minimumHeight = minHeight {
|
||||
finalHeight = max(finalHeight, minimumHeight)
|
||||
}
|
||||
|
||||
// Apply maximum constraints (expand to fill if infinity)
|
||||
if let maxW = maxWidth, case .infinity = maxW {
|
||||
if let maximumWidth = maxWidth, case .infinity = maximumWidth {
|
||||
finalWidth = context.availableWidth
|
||||
}
|
||||
if let maxH = maxHeight, case .infinity = maxH {
|
||||
if let maximumHeight = maxHeight, case .infinity = maximumHeight {
|
||||
finalHeight = context.availableHeight
|
||||
}
|
||||
|
||||
@@ -166,86 +166,6 @@ extension FlexibleFrameView: Renderable {
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Fixed Frame Modifier (Legacy)
|
||||
|
||||
/// A modifier that constrains a view to a specific width and/or height.
|
||||
///
|
||||
/// Content is aligned within the frame according to the specified alignment.
|
||||
public struct FrameModifier: ViewModifier {
|
||||
/// The desired width (nil means intrinsic width).
|
||||
public let width: Int?
|
||||
|
||||
/// The desired height (nil means intrinsic height).
|
||||
public let height: Int?
|
||||
|
||||
/// The alignment of the content within the frame.
|
||||
public let alignment: Alignment
|
||||
|
||||
public func modify(buffer: FrameBuffer, context: RenderContext) -> FrameBuffer {
|
||||
let targetWidth = width ?? buffer.width
|
||||
let targetHeight = height ?? buffer.height
|
||||
|
||||
var result: [String] = []
|
||||
|
||||
// Calculate vertical offset for alignment
|
||||
let verticalOffset: Int
|
||||
switch alignment.vertical {
|
||||
case .top:
|
||||
verticalOffset = 0
|
||||
case .center:
|
||||
verticalOffset = max(0, (targetHeight - buffer.height) / 2)
|
||||
case .bottom:
|
||||
verticalOffset = max(0, targetHeight - buffer.height)
|
||||
}
|
||||
|
||||
for row in 0..<targetHeight {
|
||||
let contentRow = row - verticalOffset
|
||||
let line: String
|
||||
if contentRow >= 0 && contentRow < buffer.lines.count {
|
||||
line = buffer.lines[contentRow]
|
||||
} else {
|
||||
line = ""
|
||||
}
|
||||
|
||||
// Align horizontally within the frame
|
||||
let aligned = alignHorizontally(
|
||||
line,
|
||||
toWidth: targetWidth,
|
||||
alignment: alignment.horizontal
|
||||
)
|
||||
result.append(aligned)
|
||||
}
|
||||
|
||||
return FrameBuffer(lines: result)
|
||||
}
|
||||
|
||||
/// Aligns a single line within the given width.
|
||||
private func alignHorizontally(
|
||||
_ line: String,
|
||||
toWidth targetWidth: Int,
|
||||
alignment: HorizontalAlignment
|
||||
) -> String {
|
||||
let visibleWidth = line.strippedLength
|
||||
|
||||
if visibleWidth >= targetWidth {
|
||||
return line
|
||||
}
|
||||
|
||||
let padding = targetWidth - visibleWidth
|
||||
|
||||
switch alignment {
|
||||
case .leading:
|
||||
return line + String(repeating: " ", count: padding)
|
||||
case .center:
|
||||
let left = padding / 2
|
||||
let right = padding - left
|
||||
return String(repeating: " ", count: left) + line + String(repeating: " ", count: right)
|
||||
case .trailing:
|
||||
return String(repeating: " ", count: padding) + line
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - View Extension
|
||||
|
||||
extension View {
|
||||
@@ -269,8 +189,17 @@ extension View {
|
||||
width: Int? = nil,
|
||||
height: Int? = nil,
|
||||
alignment: Alignment = .topLeading
|
||||
) -> ModifiedView<Self, FrameModifier> {
|
||||
modifier(FrameModifier(width: width, height: height, alignment: alignment))
|
||||
) -> some View {
|
||||
FlexibleFrameView(
|
||||
content: self,
|
||||
minWidth: width,
|
||||
idealWidth: width,
|
||||
maxWidth: width.map { .fixed($0) },
|
||||
minHeight: height,
|
||||
idealHeight: height,
|
||||
maxHeight: height.map { .fixed($0) },
|
||||
alignment: alignment
|
||||
)
|
||||
}
|
||||
|
||||
/// Sets flexible frame constraints for this view.
|
||||
|
||||
@@ -48,29 +48,29 @@ extension OverlayModifier: Renderable {
|
||||
let overlayHeight = overlayBuffer.height
|
||||
|
||||
// Calculate horizontal position
|
||||
let xOffset: Int
|
||||
let horizontalOffset: Int
|
||||
switch alignment.horizontal {
|
||||
case .leading:
|
||||
xOffset = 0
|
||||
horizontalOffset = 0
|
||||
case .center:
|
||||
xOffset = max(0, (baseWidth - overlayWidth) / 2)
|
||||
horizontalOffset = max(0, (baseWidth - overlayWidth) / 2)
|
||||
case .trailing:
|
||||
xOffset = max(0, baseWidth - overlayWidth)
|
||||
horizontalOffset = max(0, baseWidth - overlayWidth)
|
||||
}
|
||||
|
||||
// Calculate vertical position
|
||||
let yOffset: Int
|
||||
let verticalOffset: Int
|
||||
switch alignment.vertical {
|
||||
case .top:
|
||||
yOffset = 0
|
||||
verticalOffset = 0
|
||||
case .center:
|
||||
yOffset = max(0, (baseHeight - overlayHeight) / 2)
|
||||
verticalOffset = max(0, (baseHeight - overlayHeight) / 2)
|
||||
case .bottom:
|
||||
yOffset = max(0, baseHeight - overlayHeight)
|
||||
verticalOffset = max(0, baseHeight - overlayHeight)
|
||||
}
|
||||
|
||||
// Composite the overlay onto the base
|
||||
return baseBuffer.composited(with: overlayBuffer, at: (x: xOffset, y: yOffset))
|
||||
return baseBuffer.composited(with: overlayBuffer, at: (x: horizontalOffset, y: verticalOffset))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,12 @@ public enum ANSIRenderer {
|
||||
/// Reset code that clears all formatting.
|
||||
public static let reset = "\(csi)0m"
|
||||
|
||||
/// Dim/faint text style code.
|
||||
public static let dim = "\(csi)2m"
|
||||
|
||||
/// Regex pattern that matches any ANSI escape sequence.
|
||||
public static let ansiPattern = "\u{1B}\\[[0-9;]*[a-zA-Z]"
|
||||
|
||||
// MARK: - Style Rendering
|
||||
|
||||
/// Renders text with the specified style.
|
||||
|
||||
@@ -225,7 +225,7 @@ extension String {
|
||||
/// The string with all ANSI escape codes removed.
|
||||
var stripped: String {
|
||||
replacingOccurrences(
|
||||
of: "\u{1B}\\[[0-9;]*[a-zA-Z]",
|
||||
of: ANSIRenderer.ansiPattern,
|
||||
with: "",
|
||||
options: .regularExpression
|
||||
)
|
||||
|
||||
@@ -162,11 +162,10 @@ public final class Terminal: @unchecked Sendable {
|
||||
public func fillBackground(_ color: Color) {
|
||||
let size = getSize()
|
||||
let bgCode = ANSIRenderer.backgroundCode(for: color)
|
||||
let reset = ANSIRenderer.reset
|
||||
|
||||
// Move to top-left and fill each line
|
||||
var output = ANSIRenderer.moveCursor(toRow: 1, column: 1)
|
||||
let emptyLine = bgCode + String(repeating: " ", count: size.width) + reset
|
||||
let emptyLine = bgCode + String(repeating: " ", count: size.width) + ANSIRenderer.reset
|
||||
|
||||
for _ in 0..<size.height {
|
||||
output += emptyLine
|
||||
@@ -220,11 +219,11 @@ public final class Terminal: @unchecked Sendable {
|
||||
///
|
||||
/// - Returns: The read character or nil on timeout/error.
|
||||
public func readChar() -> Character? {
|
||||
var char: UInt8 = 0
|
||||
let bytesRead = read(STDIN_FILENO, &char, 1)
|
||||
var byte: UInt8 = 0
|
||||
let bytesRead = read(STDIN_FILENO, &byte, 1)
|
||||
|
||||
if bytesRead == 1 {
|
||||
return Character(UnicodeScalar(char))
|
||||
return Character(UnicodeScalar(byte))
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -36,5 +36,5 @@ public func renderOnce<Content: View>(@ViewBuilder content: () -> Content) -> In
|
||||
let view = content()
|
||||
let renderer = ViewRenderer()
|
||||
renderer.render(view)
|
||||
return 0 // TODO: Return actual line count
|
||||
return 0 // Line count not tracked by ViewRenderer
|
||||
}
|
||||
|
||||
@@ -302,7 +302,6 @@ extension Button: Renderable {
|
||||
|
||||
// Border characters (optionally colored)
|
||||
let vertical = colorizeBorder(String(style.vertical), with: color)
|
||||
let reset = "\u{1B}[0m"
|
||||
|
||||
// Top border
|
||||
let topLine = String(style.topLeft)
|
||||
@@ -313,7 +312,7 @@ extension Button: Renderable {
|
||||
// Content lines with side borders
|
||||
for line in buffer.lines {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let borderedLine = vertical + paddedLine + reset + vertical
|
||||
let borderedLine = vertical + paddedLine + ANSIRenderer.reset + vertical
|
||||
result.append(borderedLine)
|
||||
}
|
||||
|
||||
|
||||
@@ -220,7 +220,6 @@ extension ContainerView: Renderable {
|
||||
context: RenderContext
|
||||
) -> FrameBuffer {
|
||||
var lines: [String] = []
|
||||
let reset = "\u{1B}[0m"
|
||||
|
||||
// Top border (with title if present)
|
||||
let topLine: String
|
||||
@@ -255,7 +254,7 @@ extension ContainerView: Renderable {
|
||||
for line in bodyBuffer.lines {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let styledContent = applyBackground(paddedLine, background: bodyBg)
|
||||
lines.append(leftBorder + styledContent + reset + rightBorder)
|
||||
lines.append(leftBorder + styledContent + ANSIRenderer.reset + rightBorder)
|
||||
}
|
||||
|
||||
// Footer section (if present)
|
||||
@@ -274,7 +273,7 @@ extension ContainerView: Renderable {
|
||||
// Footer lines (no background - footer has its own styling)
|
||||
for line in footerBuf.lines {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
lines.append(leftBorder + paddedLine + reset + rightBorder)
|
||||
lines.append(leftBorder + paddedLine + ANSIRenderer.reset + rightBorder)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -314,7 +313,6 @@ extension ContainerView: Renderable {
|
||||
context: RenderContext
|
||||
) -> FrameBuffer {
|
||||
var lines: [String] = []
|
||||
let reset = "\u{1B}[0m"
|
||||
|
||||
// Get theme colors for block appearance
|
||||
// Header/Footer = darker background
|
||||
@@ -341,7 +339,7 @@ extension ContainerView: Renderable {
|
||||
let paddedTitle = titleStyled.padToVisibleWidth(innerWidth)
|
||||
let sideBorder = colorize("█", with: headerFooterBg)
|
||||
let styledContent = applyBackground(paddedTitle, background: headerFooterBg)
|
||||
lines.append(sideBorder + styledContent + reset + sideBorder)
|
||||
lines.append(sideBorder + styledContent + ANSIRenderer.reset + sideBorder)
|
||||
|
||||
// Header/Body separator: ▀▀▀
|
||||
// FG = header BG, BG = body BG (creates smooth transition)
|
||||
@@ -357,7 +355,7 @@ extension ContainerView: Renderable {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let sideBorder = colorize("█", with: bodyBg)
|
||||
let styledContent = applyBackground(paddedLine, background: bodyBg)
|
||||
lines.append(sideBorder + styledContent + reset + sideBorder)
|
||||
lines.append(sideBorder + styledContent + ANSIRenderer.reset + sideBorder)
|
||||
}
|
||||
|
||||
// === FOOTER SECTION (if present) ===
|
||||
@@ -374,7 +372,7 @@ extension ContainerView: Renderable {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let sideBorder = colorize("█", with: headerFooterBg)
|
||||
let styledContent = applyBackground(paddedLine, background: headerFooterBg)
|
||||
lines.append(sideBorder + styledContent + reset + sideBorder)
|
||||
lines.append(sideBorder + styledContent + ANSIRenderer.reset + sideBorder)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -411,8 +409,7 @@ extension ContainerView: Renderable {
|
||||
// ANSIRenderer.backgroundCode already returns a complete ANSI sequence
|
||||
let bgCode = ANSIRenderer.backgroundCode(for: background)
|
||||
// Replace any reset codes with reset + background to maintain the background
|
||||
let resetCode = "\u{1B}[0m"
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: resetCode, with: resetCode + bgCode)
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: ANSIRenderer.reset, with: ANSIRenderer.reset + bgCode)
|
||||
return bgCode + stringWithPersistentBg
|
||||
}
|
||||
}
|
||||
|
||||
@@ -293,11 +293,11 @@ extension Menu: Renderable {
|
||||
selectCallback?(binding.wrappedValue)
|
||||
return true
|
||||
|
||||
case .character(let char):
|
||||
case .character(let character):
|
||||
// Check for shortcut
|
||||
for (index, item) in menuItems.enumerated() {
|
||||
if let shortcut = item.shortcut,
|
||||
shortcut.lowercased() == char.lowercased() {
|
||||
shortcut.lowercased() == character.lowercased() {
|
||||
binding.wrappedValue = index
|
||||
selectCallback?(index)
|
||||
return true
|
||||
@@ -314,7 +314,7 @@ extension Menu: Renderable {
|
||||
/// The maximum width of menu items (for sizing).
|
||||
private var maxItemWidth: Int {
|
||||
items.map { item -> Int in
|
||||
let shortcutPart = item.shortcut != nil ? 4 : 4 // "[x] " or " "
|
||||
let shortcutPart = 4 // "[x] " or " " — always 4 characters wide
|
||||
return shortcutPart + item.label.count
|
||||
}.max() ?? 0
|
||||
}
|
||||
@@ -338,7 +338,6 @@ extension Menu: Renderable {
|
||||
|
||||
let innerWidth = buffer.width
|
||||
var result: [String] = []
|
||||
let reset = "\u{1B}[0m"
|
||||
|
||||
if isBlockStyle {
|
||||
// Block style: use half-blocks with special coloring
|
||||
@@ -363,8 +362,8 @@ extension Menu: Renderable {
|
||||
|
||||
// Content lines with side borders
|
||||
for (index, line) in buffer.lines.enumerated() {
|
||||
let isHeaderLine = hasHeader && index < dividerLineIndex!
|
||||
let isDividerLine = hasHeader && index == dividerLineIndex!
|
||||
let isHeaderLine = hasHeader && dividerLineIndex.map({ index < $0 }) ?? false
|
||||
let isDividerLine = hasHeader && dividerLineIndex.map({ index == $0 }) ?? false
|
||||
|
||||
if isDividerLine {
|
||||
// Header/Body separator: ▀▀▀
|
||||
@@ -376,24 +375,20 @@ extension Menu: Renderable {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let sideBorder = colorizeWithForeground("█", foreground: headerFooterBg)
|
||||
let styledContent = applyBackground(paddedLine, background: headerFooterBg)
|
||||
result.append(sideBorder + styledContent + reset + sideBorder)
|
||||
result.append(sideBorder + styledContent + ANSIRenderer.reset + sideBorder)
|
||||
} else {
|
||||
// Body line: █ borders with body background
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
let sideBorder = colorizeWithForeground("█", foreground: bodyBg)
|
||||
let styledContent = applyBackground(paddedLine, background: bodyBg)
|
||||
result.append(sideBorder + styledContent + reset + sideBorder)
|
||||
result.append(sideBorder + styledContent + ANSIRenderer.reset + sideBorder)
|
||||
}
|
||||
}
|
||||
|
||||
// Bottom border: ▀▀▀
|
||||
// FG = body background (or header if no header section), BG = App background (transparent)
|
||||
// FG = body background, BG = App background (transparent)
|
||||
let bottomLine = String(repeating: "▀", count: innerWidth + 2)
|
||||
if hasHeader {
|
||||
result.append(colorizeWithForeground(bottomLine, foreground: bodyBg))
|
||||
} else {
|
||||
result.append(colorizeWithForeground(bottomLine, foreground: bodyBg))
|
||||
}
|
||||
result.append(colorizeWithForeground(bottomLine, foreground: bodyBg))
|
||||
} else {
|
||||
// Standard style: regular box-drawing characters
|
||||
let vertical = colorizeBorder(String(style.vertical), with: color)
|
||||
@@ -414,7 +409,7 @@ extension Menu: Renderable {
|
||||
result.append(colorizeBorder(dividerLine, with: color))
|
||||
} else {
|
||||
let paddedLine = line.padToVisibleWidth(innerWidth)
|
||||
result.append(vertical + paddedLine + reset + vertical)
|
||||
result.append(vertical + paddedLine + ANSIRenderer.reset + vertical)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -448,8 +443,7 @@ extension Menu: Renderable {
|
||||
// ANSIRenderer.backgroundCode already returns a complete ANSI sequence
|
||||
let bgCode = ANSIRenderer.backgroundCode(for: background)
|
||||
// Replace any reset codes with reset + background to maintain the background
|
||||
let resetCode = "\u{1B}[0m"
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: resetCode, with: resetCode + bgCode)
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: ANSIRenderer.reset, with: ANSIRenderer.reset + bgCode)
|
||||
return bgCode + stringWithPersistentBg
|
||||
}
|
||||
|
||||
|
||||
@@ -99,22 +99,8 @@ public struct Panel<Content: View, Footer: View>: View {
|
||||
self.showFooterSeparator = showFooterSeparator
|
||||
}
|
||||
|
||||
public var body: some View {
|
||||
ContainerView(
|
||||
title: title,
|
||||
titleColor: titleColor,
|
||||
style: ContainerStyle(
|
||||
showHeaderSeparator: true,
|
||||
showFooterSeparator: showFooterSeparator,
|
||||
borderStyle: borderStyle,
|
||||
borderColor: borderColor
|
||||
),
|
||||
padding: padding
|
||||
) {
|
||||
content
|
||||
} footer: {
|
||||
footer!
|
||||
}
|
||||
public var body: Never {
|
||||
fatalError("Panel renders via Renderable")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -972,8 +972,7 @@ extension StatusBar: Renderable {
|
||||
/// Applies a background color to content, re-applying after any resets.
|
||||
private func applyBackground(_ string: String, background: Color) -> String {
|
||||
let bgCode = ANSIRenderer.backgroundCode(for: background)
|
||||
let resetCode = "\u{1B}[0m"
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: resetCode, with: resetCode + bgCode)
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: ANSIRenderer.reset, with: ANSIRenderer.reset + bgCode)
|
||||
return bgCode + stringWithPersistentBg
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,13 +11,13 @@ import TUIKit
|
||||
|
||||
/// The available demo pages in the example app.
|
||||
enum DemoPage: Int, CaseIterable {
|
||||
case menu = 0
|
||||
case textStyles = 1
|
||||
case colors = 2
|
||||
case containers = 3
|
||||
case overlays = 4
|
||||
case layout = 5
|
||||
case buttons = 6
|
||||
case menu
|
||||
case textStyles
|
||||
case colors
|
||||
case containers
|
||||
case overlays
|
||||
case layout
|
||||
case buttons
|
||||
}
|
||||
|
||||
// MARK: - App State
|
||||
|
||||
@@ -39,8 +39,8 @@ struct HeaderView: View {
|
||||
Text("TUIKit v\(tuiKitVersion)")
|
||||
.foregroundColor(.theme.foregroundTertiary)
|
||||
}
|
||||
if let sub = subtitle {
|
||||
Text(sub)
|
||||
if let subtitleText = subtitle {
|
||||
Text(subtitleText)
|
||||
.foregroundColor(.theme.foregroundSecondary)
|
||||
.italic()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,881 @@
|
||||
# TUIKit - Comprehensive Project Analysis
|
||||
|
||||
**Date:** 2026-01-30
|
||||
**Scope:** Full codebase review (66 Swift files)
|
||||
**Reviewer:** Automated Code Review Agent
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
TUIKit is a well-architected declarative Swift framework for building terminal UIs. The SwiftUI-inspired API design is clean, consistent, and idiomatic. Documentation is above average for a personal project. However, the codebase suffers from **significant code duplication** across Views and Modifiers, an **overreliance on singletons** that hinders testability, and several **thread-safety issues** with `@unchecked Sendable` types. The most impactful improvements would be extracting shared rendering logic into utilities, replacing singletons with dependency injection, and expanding test coverage for Views and Modifiers.
|
||||
|
||||
---
|
||||
|
||||
## Table of Contents
|
||||
|
||||
1. [A. Redundancies](#a-redundancies)
|
||||
2. [B. Modularization Opportunities](#b-modularization-opportunities)
|
||||
3. [C. Dead / Unused Code](#c-dead--unused-code)
|
||||
4. [D. Unused Files](#d-unused-files)
|
||||
5. [E. Documentation Gaps](#e-documentation-gaps)
|
||||
6. [F. Constant Namespacing](#f-constant-namespacing)
|
||||
7. [G. Short Variable / Constant / Parameter Names](#g-short-variable--constant--parameter-names)
|
||||
8. [H. Code Quality & Architecture](#h-code-quality--architecture)
|
||||
9. [I. Security Analysis](#i-security-analysis)
|
||||
10. [Summary Table](#summary-table)
|
||||
11. [Overall Assessment](#overall-assessment)
|
||||
|
||||
---
|
||||
|
||||
## A. Redundancies
|
||||
|
||||
### A.1 `applyBackground()` — Identical in 4 Files (High)
|
||||
|
||||
The exact same method exists in 4 files:
|
||||
|
||||
| File | Lines |
|
||||
|------|-------|
|
||||
| `Views/ContainerView.swift` | ~410-417 |
|
||||
| `Views/Menu.swift` | ~447-454 |
|
||||
| `Modifiers/BorderModifier.swift` | ~137-142 |
|
||||
| `Views/StatusBar.swift` | ~973-978 |
|
||||
|
||||
```swift
|
||||
private func applyBackground(_ string: String, background: Color) -> String {
|
||||
let bgCode = ANSIRenderer.backgroundCode(for: background)
|
||||
let resetCode = "\u{1B}[0m"
|
||||
let stringWithPersistentBg = string.replacingOccurrences(of: resetCode, with: resetCode + bgCode)
|
||||
return bgCode + stringWithPersistentBg
|
||||
}
|
||||
```
|
||||
|
||||
**Recommendation:** Extract into `ANSIRenderer.applyPersistentBackground(_:color:)` or a `String` extension.
|
||||
|
||||
---
|
||||
|
||||
### A.2 `colorize` / `colorizeBorder` — 8+ Implementations (High)
|
||||
|
||||
Almost every View and Modifier has its own colorize variant:
|
||||
|
||||
| File | Method |
|
||||
|------|--------|
|
||||
| `ContainerView.swift` | `colorize(_:with:bold:backgroundColor:)` |
|
||||
| `Menu.swift` | `colorizeWithForeground(_:foreground:)` |
|
||||
| `Menu.swift` | `colorizeWithBoth(_:foreground:background:)` |
|
||||
| `Menu.swift` | `colorizeBorder(_:with:)` |
|
||||
| `Button.swift` | `colorizeBorder(_:with:)` |
|
||||
| `BorderModifier.swift` | `colorizeWithForeground(_:foreground:)` |
|
||||
| `BorderModifier.swift` | `colorize(_:)` |
|
||||
| `StatusBar.swift` | `colorizeBorder(_:color:)` |
|
||||
| `StatusBar.swift` | `colorizeBorderWithForeground(_:foreground:)` |
|
||||
|
||||
Additionally, `StatusBar.swift`'s `colorizeBorder` and `colorizeBorderWithForeground` have **identical bodies** — only the names differ.
|
||||
|
||||
**Recommendation:** Create a single `ANSIRenderer.colorize(string:foreground:background:bold:)` method.
|
||||
|
||||
---
|
||||
|
||||
### A.3 Block-Style Border Rendering — 4x Nearly Identical (High)
|
||||
|
||||
The block-style rendering pattern (`▄`/`█`/`▀` characters for top/body/bottom) is duplicated in:
|
||||
|
||||
- `ContainerView.swift` (`renderBlockStyle`)
|
||||
- `Menu.swift` (inline in `applyBorder`)
|
||||
- `BorderModifier.swift` (`renderBlockStyle`)
|
||||
- `StatusBar.swift` (`renderBlockBordered`)
|
||||
|
||||
All follow the identical pattern:
|
||||
1. Top: `▄▄▄` with FG = container BG
|
||||
2. Content: `█ content █` with container BG
|
||||
3. Bottom: `▀▀▀` with FG = container BG
|
||||
|
||||
**Recommendation:** Extract a `BlockStyleRenderer` utility.
|
||||
|
||||
---
|
||||
|
||||
### A.4 Standard Border Rendering — 3x Nearly Identical (High)
|
||||
|
||||
`buildBorderLine` + content loop + top/bottom border in:
|
||||
|
||||
- `Button.swift` (`applyBorder`)
|
||||
- `BorderModifier.swift` (`renderStandardStyle`)
|
||||
- `Menu.swift` (standard branch of `applyBorder`)
|
||||
|
||||
**Recommendation:** Same as above — consolidate into a `BorderRenderer` utility.
|
||||
|
||||
---
|
||||
|
||||
### A.5 `let reset = "\u{1B}[0m"` — Hardcoded 8+ Times (Medium)
|
||||
|
||||
Instead of using `ANSIRenderer.reset`, the raw ANSI escape string is hardcoded in at least 8 locations:
|
||||
|
||||
- `ContainerView.swift` (lines ~223, ~317, ~414)
|
||||
- `Button.swift` (~305)
|
||||
- `BorderModifier.swift` (~74, ~105, ~139, ~238)
|
||||
- `Menu.swift` (~341)
|
||||
|
||||
**Recommendation:** Always use `ANSIRenderer.reset` constant.
|
||||
|
||||
---
|
||||
|
||||
### A.6 ThemeManager / AppearanceManager — Near-Identical Classes (High)
|
||||
|
||||
`ThemeManager` (Theme.swift) and `AppearanceManager` (Appearance.swift) share almost identical logic:
|
||||
|
||||
- Same cycling pattern
|
||||
- Same singleton access (`EnvironmentStorage.shared`, `AppState.shared`)
|
||||
- Same `set`/`cycle`/`apply` methods
|
||||
- Same bug (see [I.3](#i3-thememanager--appearancemanager-state-mismatch-bug-high))
|
||||
|
||||
**Recommendation:** Extract a generic `CyclableManager<T>` base class.
|
||||
|
||||
---
|
||||
|
||||
### A.7 Theme Structs — 5x Identical Structure (Medium)
|
||||
|
||||
`GreenPhosphorTheme`, `AmberPhosphorTheme`, `WhitePhosphorTheme`, `RedPhosphorTheme`, and `NCursesTheme` all have the exact same structure with only different color values.
|
||||
|
||||
```swift
|
||||
// Current: 5 separate structs, each ~30 lines
|
||||
public struct GreenPhosphorTheme: Theme { ... }
|
||||
public struct AmberPhosphorTheme: Theme { ... }
|
||||
// etc.
|
||||
|
||||
// Better: One generic struct with different configurations
|
||||
public struct ColorTheme: Theme {
|
||||
public init(name: String, accent: Color, ...) { ... }
|
||||
}
|
||||
```
|
||||
|
||||
`ThemeColors` (Theme.swift ~201-275) is also pure mechanical 1:1 forwarding of all Theme protocol properties — ~75 lines of boilerplate that must be manually updated when the protocol changes.
|
||||
|
||||
**Recommendation:** Replace 5 structs with one configurable `ColorTheme` struct.
|
||||
|
||||
---
|
||||
|
||||
### A.8 TupleView / ViewBuilder Boilerplate — ~500 Lines (Medium)
|
||||
|
||||
`TupleViews.swift` has 10 nearly identical structs (`TupleView2`..`TupleView10`), and `ViewBuilder.swift` has 10 nearly identical `buildBlock` overloads. `ViewRenderer.swift` has 9 copies of `Renderable` + `ChildInfoProvider` extensions.
|
||||
|
||||
Swift 6.0+ supports Parameter Packs (Variadic Generics) which could reduce all of this to a single type:
|
||||
|
||||
```swift
|
||||
struct TupleView<each V: View>: View { ... }
|
||||
```
|
||||
|
||||
**Recommendation:** Evaluate migration to Parameter Packs.
|
||||
|
||||
---
|
||||
|
||||
### A.9 Alert Preset Methods — 100% Redundant (Medium)
|
||||
|
||||
The `warning`, `error`, `info`, `success` presets are defined **twice** each (with and without actions), totaling 8 methods with nearly identical bodies (Alert.swift ~168-296). The version without actions could simply call the version with actions using `EmptyView`.
|
||||
|
||||
---
|
||||
|
||||
### A.10 Render-to-ContainerView Delegation — Repeated Pattern (Low)
|
||||
|
||||
Alert, Dialog, Panel, and Card all have the same if/else pattern in `renderToBuffer`:
|
||||
|
||||
```swift
|
||||
if let footerView = footer {
|
||||
let container = ContainerView(...) { content } footer: { footerView }
|
||||
return container.renderToBuffer(context: context)
|
||||
} else {
|
||||
let container = ContainerView(...) { content }
|
||||
return container.renderToBuffer(context: context)
|
||||
}
|
||||
```
|
||||
|
||||
**Recommendation:** Make `footer` directly passable to ContainerView (it likely already supports `nil`).
|
||||
|
||||
---
|
||||
|
||||
### A.11 `renderToBuffer` / `renderView` Duplication (Medium)
|
||||
|
||||
`Renderable.swift` has a public `renderToBuffer(view:context:)` function. `Environment.swift` has an internal `renderView(_:context:)` function with **identical logic**. The comment even acknowledges it: "Internal helper to render a view (avoids name collision with Renderable.renderToBuffer)".
|
||||
|
||||
**Recommendation:** Remove the duplicate; use the existing public function.
|
||||
|
||||
---
|
||||
|
||||
### A.12 `render()` Environment Setup — Duplicated in AppRunner (Medium)
|
||||
|
||||
`AppRunner.render()` (App.swift ~493-499) builds an environment object with 7 properties. The **identical code** exists in `renderStatusBar()` (~548-554).
|
||||
|
||||
**Recommendation:** Extract a `buildEnvironment()` helper method.
|
||||
|
||||
---
|
||||
|
||||
### A.13 `Color.lighter(by:)` / `darker(by:)` — Near Identical (Low)
|
||||
|
||||
Both methods in `Color.swift` (~216-242) have the same structure, differing only in addition vs. subtraction. Could be a shared `adjusted(by:)` method.
|
||||
|
||||
---
|
||||
|
||||
### A.14 `focusNext()` / `focusPrevious()` — Near Identical (Low)
|
||||
|
||||
`Focus.swift` (~150-183): Both methods have almost identical structure. Could be refactored to `moveFocus(direction:)`.
|
||||
|
||||
---
|
||||
|
||||
## B. Modularization Opportunities
|
||||
|
||||
### B.1 Extract `BorderRenderer` Utility (High)
|
||||
|
||||
A centralized border rendering module would eliminate the majority of code duplication:
|
||||
|
||||
```swift
|
||||
struct BorderRenderer {
|
||||
static func renderStandard(content: FrameBuffer, style: BorderStyle, color: Color, ...) -> FrameBuffer
|
||||
static func renderBlock(content: FrameBuffer, containerBg: Color, ...) -> FrameBuffer
|
||||
static func renderWithTitle(content: FrameBuffer, title: String?, ...) -> FrameBuffer
|
||||
}
|
||||
```
|
||||
|
||||
This would consolidate code from `ContainerView`, `Menu`, `Button`, `BorderModifier`, and `StatusBar`.
|
||||
|
||||
---
|
||||
|
||||
### B.2 Centralize ANSI Utilities in `ANSIRenderer` (High)
|
||||
|
||||
```swift
|
||||
extension ANSIRenderer {
|
||||
static func colorize(_ string: String, foreground: Color?, background: Color?, bold: Bool = false) -> String
|
||||
static func applyPersistentBackground(_ string: String, color: Color) -> String
|
||||
static let dimCode = "\u{1B}[2m"
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### B.3 Extract `ContainerConfig` Shared Configuration (Medium)
|
||||
|
||||
```swift
|
||||
struct ContainerConfig {
|
||||
let title: String?
|
||||
let titleColor: Color?
|
||||
let borderStyle: BorderStyle?
|
||||
let borderColor: Color?
|
||||
let padding: EdgeInsets
|
||||
let showFooterSeparator: Bool
|
||||
}
|
||||
```
|
||||
|
||||
Alert, Dialog, Card, and Panel could all use `ContainerConfig` instead of declaring the same 6 properties individually.
|
||||
|
||||
---
|
||||
|
||||
### B.4 Split `AppRunner` — God Class (Medium)
|
||||
|
||||
`AppRunner` (App.swift) has too many responsibilities: setup, rendering, event handling, cleanup, signal handling, status bar rendering, scene rendering. Should be split into:
|
||||
|
||||
- `InputHandler` — keyboard/signal event processing
|
||||
- `RenderLoop` — frame rendering pipeline
|
||||
- `SignalManager` — signal handler registration and cleanup
|
||||
|
||||
---
|
||||
|
||||
### B.5 Move `AnyView` Out of `Menu.swift` (Low)
|
||||
|
||||
`AnyView` and `.asAnyView()` extension (~Menu.swift:470-499) are general-purpose utilities that have nothing to do with Menu. They are used in `Card.swift` as well.
|
||||
|
||||
**Recommendation:** Move to a dedicated `AnyView.swift` file.
|
||||
|
||||
---
|
||||
|
||||
## C. Dead / Unused Code
|
||||
|
||||
### C.1 `BorderModifier` (Legacy) — `BorderModifier.swift:201-270` (High)
|
||||
|
||||
Explicitly marked as `// MARK: - Legacy ViewModifier (kept for compatibility)`. The new implementation is `BorderedView`. This legacy code duplicates the entire rendering logic of `BorderedView` but does **not** handle block-style rendering.
|
||||
|
||||
**Recommendation:** Remove if nothing references it, or mark with `@available(*, deprecated)`.
|
||||
|
||||
---
|
||||
|
||||
### C.2 `FrameModifier` (Legacy) — `FrameModifier.swift:174-247` (Medium)
|
||||
|
||||
Marked as "Fixed Frame Modifier (Legacy)". `FlexibleFrameView` is the active implementation. `FrameModifier` is still used by `View.frame(width:height:alignment:)`, but `FlexibleFrameView` can handle the same cases.
|
||||
|
||||
**Recommendation:** Migrate remaining callers to `FlexibleFrameView` and remove.
|
||||
|
||||
---
|
||||
|
||||
### C.3 Panel `body` Property — Dead Code (Medium)
|
||||
|
||||
`Panel.swift` implements both `body` (returns `ContainerView`) and `Renderable.renderToBuffer`. Since `Renderable` takes precedence, `body` is **never called**. The `body` also contains a force-unwrap (`footer!`) that would crash if ever executed.
|
||||
|
||||
**Recommendation:** Remove the `body` implementation or mark it clearly as unreachable.
|
||||
|
||||
---
|
||||
|
||||
### C.4 Common Preference Keys — Likely Unused (Low)
|
||||
|
||||
`Preferences.swift` defines `NavigationTitleKey`, `TabBadgeKey`, and `AnchorPreferenceKey` as "Common Preference Keys", but no internal Views use them. These appear to be forward-looking definitions with no current consumers.
|
||||
|
||||
**Recommendation:** Verify if any external code uses these. If not, remove them or add `// TODO: Used by future navigation/tab features` comments.
|
||||
|
||||
---
|
||||
|
||||
### C.5 TODO Placeholder — `TUIKit.swift` (Low)
|
||||
|
||||
```swift
|
||||
return 0 // TODO: Return actual line count
|
||||
```
|
||||
|
||||
Either implement the actual line count or change the return type.
|
||||
|
||||
---
|
||||
|
||||
### C.6 Menu: Pointless Ternary (Low)
|
||||
|
||||
`Menu.swift` (~316-319):
|
||||
|
||||
```swift
|
||||
let shortcutPart = item.shortcut != nil ? 4 : 4 // "[x] " or " "
|
||||
```
|
||||
|
||||
Both branches return `4`. The ternary operator is completely meaningless.
|
||||
|
||||
---
|
||||
|
||||
### C.7 Menu: Identical if/else Branches (Low)
|
||||
|
||||
`Menu.swift` (~392-396) in block-style bottom border:
|
||||
|
||||
```swift
|
||||
if hasHeader {
|
||||
result.append(colorizeWithForeground(bottomLine, foreground: bodyBg))
|
||||
} else {
|
||||
result.append(colorizeWithForeground(bottomLine, foreground: bodyBg))
|
||||
}
|
||||
```
|
||||
|
||||
Both branches are **identical**.
|
||||
|
||||
---
|
||||
|
||||
### C.8 `_ = self // Silence warning` — Anti-Pattern (Low)
|
||||
|
||||
`App.swift` (~453): This capture-silencing pattern should be resolved properly (e.g., remove `[weak self]` if not needed, or use `self` meaningfully).
|
||||
|
||||
---
|
||||
|
||||
## D. Unused Files
|
||||
|
||||
No entirely unused files were identified. All `.swift` files contribute to either the framework, the example app, or the test suite. The closest candidates are:
|
||||
|
||||
- **`UserDefaultsStorage.swift`**: Contains a large Linux compatibility layer that duplicates parts of `AppStorage.swift`. Could potentially be merged.
|
||||
- **`PrimitiveViews.swift`**: All views here are minimal primitives (`EmptyView`, `AnyView`, `Divider`, etc.). While not "unused", some like `AnyView` also exist in `Menu.swift` (see B.5).
|
||||
|
||||
---
|
||||
|
||||
## E. Documentation Gaps
|
||||
|
||||
### E.1 Public Types / Properties Missing `///` Doc Comments
|
||||
|
||||
| File | Element | Severity |
|
||||
|------|---------|----------|
|
||||
| `BorderModifier.swift` | `BorderedView` properties (`content`, `style`, `color`) | Medium |
|
||||
| `FrameModifier.swift` | `FlexibleFrameView` properties (`content`, `minWidth`, `maxWidth`, etc.) | Medium |
|
||||
| `DimmedModifier.swift` | `content` property | Low |
|
||||
| `KeyPressModifier.swift` | `content`, `keys`, `handler` properties | Low |
|
||||
| `OverlayModifier.swift` | `base`, `overlay`, `alignment` properties | Low |
|
||||
| `StatusBarItemsModifier.swift` | `content`, `items`, `context` properties | Low |
|
||||
| `ViewModifier.swift` | `ModifiedView.content` and `ModifiedView.modifier` — public but minimal docs | Medium |
|
||||
|
||||
### E.2 Complex Logic Without Inline Comments
|
||||
|
||||
| File | Area | Recommendation |
|
||||
|------|------|----------------|
|
||||
| `KeyEvent.swift` | Byte-level escape sequence parsing | Add comments explaining each escape sequence format |
|
||||
| `FrameBuffer.swift` | `composited(with:at:)` and `insertOverlay` | Document how ANSI codes are preserved/stripped |
|
||||
| `ViewRenderer.swift` | `resolveChildInfos` and TupleView extensions | Document the rendering pipeline flow |
|
||||
| `App.swift` | Signal handler setup (~632-646) | Document async-signal-safety constraints |
|
||||
|
||||
### E.3 Missing Developer Guides for Contributors
|
||||
|
||||
- No `CONTRIBUTING.md` or developer setup guide
|
||||
- No architectural diagram showing the rendering pipeline
|
||||
- No guide explaining the `body` vs. `Renderable` dual rendering system
|
||||
- No documentation on how to add a new View type
|
||||
|
||||
### E.4 Example App: Missing Demonstrations
|
||||
|
||||
The example app has significant gaps in demonstrating framework capabilities:
|
||||
|
||||
| Missing Demo | Framework Feature |
|
||||
|------|------|
|
||||
| Interactive buttons | All button actions are empty `{ }` |
|
||||
| `Color.hex()` | Supported but not shown |
|
||||
| `Color.palette()` | Supported but not shown |
|
||||
| `.background()` modifier | Only used once for black |
|
||||
| `.modal()` helper | Page uses manual `.dimmed().overlay()` instead |
|
||||
| State mutation | No counter, toggle, or reactive demo |
|
||||
| Nested containers | Card in Panel, etc. |
|
||||
| `ZStack` | Not demonstrated |
|
||||
|
||||
---
|
||||
|
||||
## F. Constant Namespacing
|
||||
|
||||
### F.1 ANSI Escape Codes in `KeyEvent.swift` — 60+ Magic Hex Values (High)
|
||||
|
||||
The entire key event parsing system uses raw hex values:
|
||||
|
||||
```swift
|
||||
// Scattered throughout KeyEvent.swift
|
||||
if bytes[0] == 0x1B { ... } // Escape
|
||||
if bytes[0] == 0x0D { ... } // Carriage return
|
||||
if bytes[0] == 0x7F { ... } // Delete
|
||||
// 0x41, 0x42, 0x43, 0x44, 0x48, 0x46, 0x7E, 0x5B, 0x4F, ...
|
||||
```
|
||||
|
||||
**Recommendation:**
|
||||
|
||||
```swift
|
||||
private enum ASCIIByte {
|
||||
static let escape: UInt8 = 0x1B
|
||||
static let carriageReturn: UInt8 = 0x0D
|
||||
static let lineFeed: UInt8 = 0x0A
|
||||
static let tab: UInt8 = 0x09
|
||||
static let delete: UInt8 = 0x7F
|
||||
static let backspace: UInt8 = 0x08
|
||||
static let openBracket: UInt8 = 0x5B
|
||||
static let letterO: UInt8 = 0x4F
|
||||
static let tilde: UInt8 = 0x7E
|
||||
// Arrow keys
|
||||
static let arrowUp: UInt8 = 0x41
|
||||
static let arrowDown: UInt8 = 0x42
|
||||
static let arrowRight: UInt8 = 0x43
|
||||
static let arrowLeft: UInt8 = 0x44
|
||||
static let home: UInt8 = 0x48
|
||||
static let end: UInt8 = 0x46
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### F.2 ANSI Style Codes in `ANSIRenderer.swift` (Medium)
|
||||
|
||||
```swift
|
||||
// Current: magic strings
|
||||
codes.append("1") // bold
|
||||
codes.append("2") // dim
|
||||
codes.append("3") // italic
|
||||
// ...
|
||||
```
|
||||
|
||||
**Recommendation:**
|
||||
|
||||
```swift
|
||||
private enum ANSIStyleCode {
|
||||
static let bold = "1"
|
||||
static let dim = "2"
|
||||
static let italic = "3"
|
||||
static let underline = "4"
|
||||
static let blink = "5"
|
||||
static let inverse = "7"
|
||||
static let strikethrough = "9"
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### F.3 Block Characters Scattered Across Files (Medium)
|
||||
|
||||
The characters `"▄"`, `"▀"`, `"█"` are used as raw string literals in `ContainerView`, `Menu`, `BorderModifier`, and `StatusBar`.
|
||||
|
||||
**Recommendation:**
|
||||
|
||||
```swift
|
||||
enum BlockCharacters {
|
||||
static let upperHalf = "▄"
|
||||
static let lowerHalf = "▀"
|
||||
static let fullBlock = "█"
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### F.4 Magic Numbers in Layout (Low)
|
||||
|
||||
| File | Code | Meaning |
|
||||
|------|------|---------|
|
||||
| `ContainerView.swift` | `$0.count + 4` | Title padding width — unclear why 4 |
|
||||
| `Menu.swift` | `+ 2` | Content padding |
|
||||
| `Button.swift` | `horizontalPadding: 2` | Default button padding |
|
||||
| `Alert.swift` | `EdgeInsets(horizontal: 2, vertical: 1)` | Standard alert padding |
|
||||
| `Dialog.swift` | `EdgeInsets(horizontal: 2, vertical: 1)` | Same as alert — should be shared constant |
|
||||
| `StatusBar.swift` | `- 2` | Border width subtraction |
|
||||
| `BorderModifier.swift` | `- 2` | Border width subtraction |
|
||||
|
||||
**Recommendation:**
|
||||
|
||||
```swift
|
||||
enum LayoutConstants {
|
||||
static let borderWidth = 2
|
||||
static let defaultContainerPadding = EdgeInsets(horizontal: 2, vertical: 1)
|
||||
static let defaultButtonPadding = EdgeInsets(horizontal: 2, vertical: 0)
|
||||
static let statusBarItemSeparator = " "
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### F.5 `DemoPage` Explicit Raw Values (Low)
|
||||
|
||||
`AppState.swift` (~13-21): `case menu = 0, textStyles = 1, ...` — Int-based enums number automatically. The explicit values are redundant.
|
||||
|
||||
---
|
||||
|
||||
## G. Short Variable / Constant / Parameter Names
|
||||
|
||||
### G.1 Core Framework — Color.swift HSL Conversion
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `Color.swift` | `hsl()` method | `h` | `normalizedHue` |
|
||||
| `Color.swift` | `hsl()` method | `s` | `normalizedSaturation` |
|
||||
| `Color.swift` | `hsl()` method | `l` | `normalizedLightness` |
|
||||
| `Color.swift` | `hsl()` method | `q` | `chromaFactor` |
|
||||
| `Color.swift` | `hsl()` method | `p` | `luminanceFactor` |
|
||||
| `Color.swift` | `hueToRGB` function | `p` (param) | `luminance` |
|
||||
| `Color.swift` | `hueToRGB` function | `q` (param) | `chroma` |
|
||||
| `Color.swift` | `hueToRGB` function | `t` (param) | `hueComponent` |
|
||||
| `Color.swift` | `hueToRGB` function | `t` (shadow var) | `adjustedHue` |
|
||||
|
||||
### G.2 Core Framework — TupleViews.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `TupleViews.swift` | All 10 structs | `V0`..`V9` (generics) | `View0`..`View9` |
|
||||
| `TupleViews.swift` | All 10 structs | `v0`..`v9` (properties) | `view0`..`view9` |
|
||||
|
||||
*Note: SwiftUI itself uses short generic names for TupleViews. This is an accepted Swift convention for result builders. Pragmatically acceptable.*
|
||||
|
||||
### G.3 Core Framework — ViewBuilder.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `ViewBuilder.swift` | All `buildBlock` methods | `C0`..`C9` (generics) | `View0`..`View9` |
|
||||
| `ViewBuilder.swift` | All `buildBlock` methods | `c0`..`c9` (params) | `view0`..`view9` |
|
||||
|
||||
*Same note as TupleViews — standard Swift convention for result builders.*
|
||||
|
||||
### G.4 Modifiers — FrameModifier.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `FrameModifier.swift` | Local bindings | `maxW` | `maximumWidth` |
|
||||
| `FrameModifier.swift` | Local bindings | `maxH` | `maximumHeight` |
|
||||
| `FrameModifier.swift` | Local bindings | `minW` | `minimumWidth` |
|
||||
| `FrameModifier.swift` | Local bindings | `minH` | `minimumHeight` |
|
||||
|
||||
### G.5 Modifiers — OverlayModifier.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `OverlayModifier.swift` | Overlay positioning | `xOffset` | `horizontalOffset` |
|
||||
| `OverlayModifier.swift` | Overlay positioning | `yOffset` | `verticalOffset` |
|
||||
|
||||
### G.6 Modifiers — BackgroundModifier.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `BackgroundModifier.swift` | Switch cases | `ansi` | `ansiColor` |
|
||||
| `BackgroundModifier.swift` | Switch case | `index` (in `.palette256`) | `paletteIndex` |
|
||||
|
||||
### G.7 Rendering — Terminal.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `Terminal.swift` | readByte method | `char` | `readByte` |
|
||||
|
||||
### G.8 Example App — HeaderView.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `HeaderView.swift` | `if let sub = subtitle` | `sub` | `subtitleText` |
|
||||
|
||||
### G.9 Views — Menu.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `Menu.swift` | Key event handling | `char` | `characterValue` |
|
||||
|
||||
### G.10 Core Framework — Focus.swift
|
||||
|
||||
| File | Context | Name | Suggested Name |
|
||||
|------|---------|------|----------------|
|
||||
| `Focus.swift` | Focus ID binding | `id` | `focusedIdentifier` |
|
||||
|
||||
*Note: `id` in this context is reasonably clear. Borderline case.*
|
||||
|
||||
---
|
||||
|
||||
## H. Code Quality & Architecture
|
||||
|
||||
### H.1 Singleton Overuse — 8+ `shared` Instances (Critical)
|
||||
|
||||
The framework relies on at least 8 singletons:
|
||||
|
||||
| Singleton | File |
|
||||
|-----------|------|
|
||||
| `AppState.shared` | `State.swift` |
|
||||
| `EnvironmentStorage.shared` | `Environment.swift` |
|
||||
| `PreferenceStorage.shared` | `Preferences.swift` |
|
||||
| `Terminal.shared` | `Terminal.swift` |
|
||||
| `StorageManager.shared` | `AppStorage.swift` |
|
||||
| `KeyEventDispatcher.shared` | `KeyEvent.swift` |
|
||||
| `FocusManager.shared` (implicit) | `Focus.swift` |
|
||||
| `LifecycleTracker.shared` | `LifecycleModifier.swift` |
|
||||
|
||||
**Impact:** Unit testing is extremely difficult. Tests cannot inject mock implementations. Test suites that modify `EnvironmentStorage.shared` leak state between tests. Parallel test execution is impossible.
|
||||
|
||||
**Recommendation:** Introduce a `TUIContext` object that holds all shared state and is passed through the rendering pipeline. Views access it via `@Environment` rather than singletons.
|
||||
|
||||
---
|
||||
|
||||
### H.2 Dual Rendering System (`body` vs. `Renderable`) — Inconsistent (Medium)
|
||||
|
||||
Some Views implement `body` (Box, Spacer), some implement `Renderable` (Alert, Dialog, Button), and some implement **both** (Panel — where `body` is dead code). The relationship between these two systems is not documented.
|
||||
|
||||
**Recommendation:** Document the contract clearly. If a View implements `Renderable`, `body` should either not exist or be explicitly marked as unreachable.
|
||||
|
||||
---
|
||||
|
||||
### H.3 `AppRunner` is a God Class (Medium)
|
||||
|
||||
`AppRunner` (App.swift) handles: initialization, rendering, event dispatching, cleanup, signal handling, status bar rendering, and scene rendering. This violates Single Responsibility Principle.
|
||||
|
||||
**Recommendation:** Split into `InputHandler`, `RenderLoop`, and `SignalManager`.
|
||||
|
||||
---
|
||||
|
||||
### H.4 Preference Callback Accumulation (Medium)
|
||||
|
||||
`PreferenceStorage` (`Preferences.swift`): Callbacks are registered during rendering (`onPreferenceChange`) and triggered immediately. Callbacks appear to be re-registered on every render pass, but `clearCallbacks()` doesn't seem to be called in the render loop. This means the `callbacks` dictionary grows with each render cycle.
|
||||
|
||||
**Recommendation:** Clear callbacks at the start of each render cycle, or use a Set-based deduplication.
|
||||
|
||||
---
|
||||
|
||||
### H.5 `FocusState` Bypasses Environment System (Low)
|
||||
|
||||
`FocusState` (`Focus.swift`) directly accesses `EnvironmentStorage.shared` instead of going through the `@Environment` property wrapper. This couples it to the singleton and bypasses the designed abstraction.
|
||||
|
||||
---
|
||||
|
||||
### H.6 `SceneStorage` Creates New Instance on Every Access (Low)
|
||||
|
||||
`AppStorage.swift`: The `sceneStorage` static property creates a new `JSONFileStorage` instance on **every access**. Every read/write creates a new storage object and re-reads the file from disk.
|
||||
|
||||
**Recommendation:** Cache the instance (lazy static or singleton pattern).
|
||||
|
||||
---
|
||||
|
||||
### H.7 `FrameBuffer` Regex Performance (Medium)
|
||||
|
||||
`FrameBuffer.swift`: `strippedLength` and `stripped` compile a regular expression on **every call** using `.regularExpression` mode. In a rendering pipeline that calls this for every line on every frame, this is a performance bottleneck.
|
||||
|
||||
**Recommendation:** Use a precompiled static `NSRegularExpression` or Swift `Regex`.
|
||||
|
||||
---
|
||||
|
||||
### H.8 Test Coverage Gaps (High)
|
||||
|
||||
Major untested areas:
|
||||
|
||||
| Area | Status |
|
||||
|------|--------|
|
||||
| `Card` rendering | Not tested |
|
||||
| `Box` rendering | Not tested |
|
||||
| `Panel` rendering | Not tested |
|
||||
| `ContainerView` rendering | Not tested |
|
||||
| `.frame()` modifier | Not tested |
|
||||
| `.padding()` modifier | Not tested |
|
||||
| `.border()` modifier | Not tested |
|
||||
| `.background()` modifier | Not tested |
|
||||
| Text style rendering (bold, italic, etc.) | Not tested at rendering level |
|
||||
| Menu navigation (up/down keys) | Not tested |
|
||||
| Menu `onSelect` callback | Not tested |
|
||||
| Nested stacks | Not tested |
|
||||
| Theme color rendering | Not tested |
|
||||
|
||||
Existing tests are often smoke tests (`buffer.height > 2`, `!buffer.isEmpty`) rather than structural assertions.
|
||||
|
||||
---
|
||||
|
||||
### H.9 Package.swift Configuration (Low)
|
||||
|
||||
- **`macOS(.v10_15)`** is outdated. Swift 6 tools-version effectively requires macOS 13+. macOS 10.15 (Catalina) is end-of-life.
|
||||
- **Missing `swiftLanguageVersions`** setting.
|
||||
- **No CI for Linux** despite claiming Linux support.
|
||||
|
||||
---
|
||||
|
||||
## I. Security Analysis
|
||||
|
||||
### I.1 Thread-Safety: `@unchecked Sendable` Without Locks (Critical)
|
||||
|
||||
| File | Type | Mutable State |
|
||||
|------|------|---------------|
|
||||
| `LifecycleModifier.swift` | `LifecycleTracker` | `appearedTokens`, `visibleTokens`, `currentRenderTokens` |
|
||||
| `LifecycleModifier.swift` | `DisappearCallbackStorage` | `callbacks` dictionary |
|
||||
| `LifecycleModifier.swift` | `TaskStorage` | `tasks` dictionary |
|
||||
| `AppState.swift` (example) | `ExampleAppState` | `currentPage`, `menuSelection` |
|
||||
|
||||
All are marked `@unchecked Sendable` with shared mutable state but **no synchronization mechanism** (no `NSLock`, no actor isolation, no dispatch queue). Only `TokenGenerator` correctly uses a lock.
|
||||
|
||||
**Recommendation:** Either make these `actor` types or add `NSLock`/`os_unfair_lock` protection.
|
||||
|
||||
---
|
||||
|
||||
### I.2 Signal Handler Safety (High)
|
||||
|
||||
`App.swift` (~632-646): The SIGINT signal handler calls `Terminal.shared.disableRawMode()`, `Terminal.shared.exitAlternateScreen()`, and `exit(0)`. Signal handlers should only call async-signal-safe functions. `print`, `fflush`, and writing ANSI escape codes are **not** guaranteed async-signal-safe.
|
||||
|
||||
**Recommendation:** Set a flag in the signal handler and handle cleanup in the main loop, or use `sigaction` with `SA_RESETHAND` and minimal handler.
|
||||
|
||||
---
|
||||
|
||||
### I.3 ThemeManager / AppearanceManager State Mismatch Bug (High)
|
||||
|
||||
When `setTheme()` is called with a theme not in `availableThemes`:
|
||||
1. `currentIndex` is set to `0`
|
||||
2. But `environment.theme` receives the **actual passed theme**
|
||||
3. After this, `currentTheme` (which reads from `availableThemes[currentIndex]`) returns the **wrong theme**
|
||||
|
||||
The same bug exists in `AppearanceManager.setAppearance()`.
|
||||
|
||||
**Recommendation:** Either add the unknown theme to `availableThemes`, or don't update `environment.theme` when the theme isn't found.
|
||||
|
||||
---
|
||||
|
||||
### I.4 Force Unwraps (Medium)
|
||||
|
||||
| File | Location | Risk |
|
||||
|------|----------|------|
|
||||
| `Panel.swift` | `body` property: `footer!` | Crash if `footer` is nil (dead code, but dangerous) |
|
||||
| `Menu.swift` | `dividerLineIndex!` | Crash if nil (currently guarded by `hasHeader` check, but fragile) |
|
||||
| `StatusBarTests.swift` | 6 force unwraps (`saveIndex!`, `quitIndex!`, etc.) | Tests crash instead of failing gracefully |
|
||||
|
||||
---
|
||||
|
||||
### I.5 `needsRerender` Global Variable — Data Race (Medium)
|
||||
|
||||
`App.swift` (~409): `nonisolated(unsafe) var needsRerender` is a global mutable Bool written by a signal handler and read by the main loop. This is technically a data race (even if practically harmless for Bool). Should use `Atomic<Bool>` or `os_unfair_lock`.
|
||||
|
||||
---
|
||||
|
||||
### I.6 Silent Error Swallowing in Storage (Low)
|
||||
|
||||
`AppStorage.swift`: Both `setValue` (~87-88) and `loadFromDisk` (~122-124) silently catch and ignore encoding/decoding errors. Failed persistence is never reported.
|
||||
|
||||
**Recommendation:** Add at minimum `#if DEBUG` logging for failed operations.
|
||||
|
||||
---
|
||||
|
||||
### I.7 `Terminal.readLine()` Misleading in Raw Mode (Low)
|
||||
|
||||
`Terminal.swift` (~274-276): `readLine()` delegates to `Swift.readLine()`, which doesn't work in raw mode (raw mode disables line-based input). This method is misleading and could cause hangs.
|
||||
|
||||
---
|
||||
|
||||
### I.8 `deinit` on Singleton — Never Called (Low)
|
||||
|
||||
`Terminal.swift`: The `deinit` disables raw mode, but since `Terminal` is a singleton, `deinit` is never called. Cleanup relies entirely on `cleanup()` being called by `AppRunner`. The `deinit` gives a false sense of safety.
|
||||
|
||||
---
|
||||
|
||||
## Summary Table
|
||||
|
||||
| # | Finding | Category | Severity | File(s) |
|
||||
|---|---------|----------|----------|---------|
|
||||
| A.1 | `applyBackground()` 4x identical | Redundancy | High | ContainerView, Menu, BorderModifier, StatusBar |
|
||||
| A.2 | `colorize` variants 8+ copies | Redundancy | High | Multiple |
|
||||
| A.3 | Block-style rendering 4x identical | Redundancy | High | ContainerView, Menu, BorderModifier, StatusBar |
|
||||
| A.4 | Standard border rendering 3x identical | Redundancy | High | Button, BorderModifier, Menu |
|
||||
| A.5 | `"\u{1B}[0m"` hardcoded 8+ times | Redundancy | Medium | Multiple |
|
||||
| A.6 | ThemeManager/AppearanceManager near-identical | Redundancy | High | Theme.swift, Appearance.swift |
|
||||
| A.7 | 5 Theme structs identical structure | Redundancy | Medium | Theme.swift |
|
||||
| A.8 | TupleView/ViewBuilder ~500 lines boilerplate | Redundancy | Medium | TupleViews, ViewBuilder, ViewRenderer |
|
||||
| A.9 | Alert presets 100% redundant | Redundancy | Medium | Alert.swift |
|
||||
| A.10 | ContainerView delegation pattern repeated | Redundancy | Low | Alert, Dialog, Panel, Card |
|
||||
| A.11 | `renderToBuffer` / `renderView` duplicate | Redundancy | Medium | Renderable.swift, Environment.swift |
|
||||
| A.12 | AppRunner environment setup duplicated | Redundancy | Medium | App.swift |
|
||||
| A.13 | `lighter(by:)` / `darker(by:)` near identical | Redundancy | Low | Color.swift |
|
||||
| A.14 | `focusNext()` / `focusPrevious()` near identical | Redundancy | Low | Focus.swift |
|
||||
| B.1 | Extract `BorderRenderer` utility | Modularization | High | Multiple |
|
||||
| B.2 | Centralize ANSI utilities | Modularization | High | Multiple |
|
||||
| B.3 | Extract `ContainerConfig` | Modularization | Medium | Alert, Dialog, Panel, Card |
|
||||
| B.4 | Split `AppRunner` god class | Modularization | Medium | App.swift |
|
||||
| B.5 | Move `AnyView` out of `Menu.swift` | Modularization | Low | Menu.swift |
|
||||
| C.1 | `BorderModifier` legacy code | Dead Code | High | BorderModifier.swift |
|
||||
| C.2 | `FrameModifier` legacy code | Dead Code | Medium | FrameModifier.swift |
|
||||
| C.3 | Panel `body` dead code | Dead Code | Medium | Panel.swift |
|
||||
| C.4 | Common Preference Keys likely unused | Dead Code | Low | Preferences.swift |
|
||||
| C.5 | TODO placeholder return 0 | Dead Code | Low | TUIKit.swift |
|
||||
| C.6 | Menu pointless ternary `4 : 4` | Dead Code | Low | Menu.swift |
|
||||
| C.7 | Menu identical if/else branches | Dead Code | Low | Menu.swift |
|
||||
| C.8 | `_ = self` anti-pattern | Dead Code | Low | App.swift |
|
||||
| E.1 | Public types missing doc comments | Documentation | Medium | Multiple |
|
||||
| E.2 | Complex logic without inline comments | Documentation | Medium | KeyEvent, FrameBuffer, ViewRenderer |
|
||||
| E.3 | Missing contributor documentation | Documentation | Medium | Project root |
|
||||
| E.4 | Example app missing demonstrations | Documentation | Low | TUIKitExample |
|
||||
| F.1 | 60+ magic hex values in KeyEvent | Constants | High | KeyEvent.swift |
|
||||
| F.2 | ANSI style codes as magic strings | Constants | Medium | ANSIRenderer.swift |
|
||||
| F.3 | Block characters scattered | Constants | Medium | Multiple |
|
||||
| F.4 | Magic numbers in layout | Constants | Low | Multiple |
|
||||
| G.1-10 | Short variable names (see section G) | Naming | Medium | Multiple |
|
||||
| H.1 | 8+ singletons hinder testability | Architecture | Critical | Multiple |
|
||||
| H.2 | Dual rendering system inconsistent | Architecture | Medium | Multiple |
|
||||
| H.3 | AppRunner god class | Architecture | Medium | App.swift |
|
||||
| H.4 | Preference callback accumulation | Architecture | Medium | Preferences.swift |
|
||||
| H.7 | FrameBuffer regex performance | Architecture | Medium | FrameBuffer.swift |
|
||||
| H.8 | Major test coverage gaps | Architecture | High | Tests/ |
|
||||
| H.9 | Package.swift outdated macOS minimum | Architecture | Low | Package.swift |
|
||||
| I.1 | `@unchecked Sendable` without locks | Security | Critical | LifecycleModifier, AppState |
|
||||
| I.2 | Signal handler not async-signal-safe | Security | High | App.swift |
|
||||
| I.3 | ThemeManager/AppearanceManager state bug | Security | High | Theme.swift, Appearance.swift |
|
||||
| I.4 | Force unwraps | Security | Medium | Panel, Menu, Tests |
|
||||
| I.5 | `needsRerender` data race | Security | Medium | App.swift |
|
||||
| I.6 | Silent error swallowing in storage | Security | Low | AppStorage.swift |
|
||||
| I.7 | `readLine()` misleading in raw mode | Security | Low | Terminal.swift |
|
||||
| I.8 | Singleton deinit never called | Security | Low | Terminal.swift |
|
||||
|
||||
---
|
||||
|
||||
## Overall Assessment
|
||||
|
||||
### Strengths
|
||||
|
||||
1. **Clean API Design** — The SwiftUI-inspired declarative API is well-designed, consistent, and idiomatic Swift.
|
||||
2. **Zero Dependencies** — Pure Swift with no C library dependencies is a strong selling point.
|
||||
3. **Good Documentation Foundation** — DocC catalog with articles, hosted on GitHub Pages with custom domain.
|
||||
4. **Comprehensive Theme System** — 5 built-in themes with proper protocol-based extensibility.
|
||||
5. **Solid Focus Management** — FocusManager with keyboard navigation, wrapping, and disabled element support.
|
||||
6. **Well-Tested Focus & StatusBar** — These two areas have thorough test coverage.
|
||||
7. **Proper Environment System** — `@Environment`, `@State`, `@AppStorage` following SwiftUI patterns.
|
||||
|
||||
### Weaknesses
|
||||
|
||||
1. **Massive Code Duplication** — Border rendering, colorization, and background application are copied across 4-8 files. This is the single biggest maintenance burden.
|
||||
2. **Singleton Addiction** — 8+ global shared instances make the framework nearly untestable and fragile for concurrent use.
|
||||
3. **Thread-Safety Gaps** — Multiple `@unchecked Sendable` types with no synchronization under Swift 6 strict concurrency.
|
||||
4. **Test Coverage Holes** — Views, Modifiers, and the rendering pipeline are largely untested. Most tests are smoke tests.
|
||||
5. **Legacy Code Retained** — `BorderModifier` and `FrameModifier` legacy implementations add confusion without adding value.
|
||||
6. **Signal Handler Safety** — The SIGINT handler is not async-signal-safe and could crash in edge cases.
|
||||
|
||||
### Priority Recommendations
|
||||
|
||||
1. **Extract `BorderRenderer` + `ANSIRenderer.colorize()`** — Eliminates ~60% of all duplication.
|
||||
2. **Fix `@unchecked Sendable` types** — Add locks or convert to actors.
|
||||
3. **Fix ThemeManager/AppearanceManager state bug** — Actual functional bug.
|
||||
4. **Remove legacy `BorderModifier` and `FrameModifier`** — Reduce confusion.
|
||||
5. **Add constants namespacing for hex values and ANSI codes** — Improve readability.
|
||||
6. **Expand test coverage** — Especially for Views and Modifiers at the rendering level.
|
||||
7. **Plan singleton migration** — Introduce `TUIContext` for dependency injection (longer-term).
|
||||
Reference in New Issue
Block a user