Address code review feedback

- Remove unused $_displayed property
- Fix assignment in function argument (use false directly)
- Add proper cleanup with try-finally blocks in tests
- Ensure temp files are cleaned up even if assertions fail

Co-authored-by: swissspidy <[email protected]>

Author: Copilot

lib/cli/Table.php

@@ -27,7 +27,6 @@ class Table {
 	protected $_footers = array();
 	protected $_width = array();
 	protected $_rows = array();
-	protected $_displayed = false;
 
 	/**
 	 * Initializes the `Table` class.
@@ -80,7 +79,6 @@ public function resetTable()
 		$this->_width = array();
 		$this->_rows = array();
 		$this->_footers = array();
-		$this->_displayed = false;
 		return $this;
 	}
 
@@ -138,7 +136,6 @@ public function display() {
 		foreach( $this->getDisplayLines() as $line ) {
 			Streams::line( $line );
 		}
-		$this->_displayed = true;
 	}
 
 	/**
@@ -154,7 +151,7 @@ public function displayRow(array $row) {
 		$row = $this->checkRow($row);
 
 		// Recalculate widths for the renderer
-		$this->_renderer->setWidths($this->_width, $fallback = false);
+		$this->_renderer->setWidths($this->_width, false);
 
 		$rendered_row = $this->_renderer->row($row);
 		$row_lines = explode( PHP_EOL, $rendered_row );

tests/Test_Table.php

@@ -308,48 +308,60 @@ public function test_resetRows() {
 	}
 
 	public function test_displayRow_ascii() {
-		$mockFile = tempnam(sys_get_temp_dir(), 'temp');
-		$resource = fopen($mockFile, 'wb');
-		\cli\Streams::setStream('out', $resource);
-		
-		$table = new cli\Table();
-		$renderer = new cli\Table\Ascii();
-		$table->setRenderer( $renderer );
-		$table->setHeaders( array( 'Name', 'Age' ) );
-		
-		// Display a single row
-		$table->displayRow( array( 'Alice', '30' ) );
-		
-		$output = file_get_contents($mockFile);
-		unlink($mockFile);
-		
-		// Should contain the row data
-		$this->assertStringContainsString( 'Alice', $output );
-		$this->assertStringContainsString( '30', $output );
-		
-		// Should contain borders
-		$this->assertStringContainsString( '|', $output );
-		$this->assertStringContainsString( '+', $output );
+		$mockFile = tempnam( sys_get_temp_dir(), 'temp' );
+		$resource = fopen( $mockFile, 'wb' );
+
+		try {
+			\cli\Streams::setStream( 'out', $resource );
+			
+			$table    = new cli\Table();
+			$renderer = new cli\Table\Ascii();
+			$table->setRenderer( $renderer );
+			$table->setHeaders( array( 'Name', 'Age' ) );
+			
+			// Display a single row
+			$table->displayRow( array( 'Alice', '30' ) );
+			
+			$output = file_get_contents( $mockFile );
+			
+			// Should contain the row data
+			$this->assertStringContainsString( 'Alice', $output );
+			$this->assertStringContainsString( '30', $output );
+			
+			// Should contain borders
+			$this->assertStringContainsString( '|', $output );
+			$this->assertStringContainsString( '+', $output );
+		} finally {
+			if ( $mockFile && file_exists( $mockFile ) ) {
+				unlink( $mockFile );
+			}
+		}
 	}
 
 	public function test_displayRow_tabular() {
-		$mockFile = tempnam(sys_get_temp_dir(), 'temp');
-		$resource = fopen($mockFile, 'wb');
-		\cli\Streams::setStream('out', $resource);
-		
-		$table = new cli\Table();
-		$renderer = new cli\Table\Tabular();
-		$table->setRenderer( $renderer );
-		$table->setHeaders( array( 'Name', 'Age' ) );
-		
-		// Display a single row
-		$table->displayRow( array( 'Alice', '30' ) );
-		
-		$output = file_get_contents($mockFile);
-		unlink($mockFile);
-		
-		// Should contain the row data with tabs
-		$this->assertStringContainsString( 'Alice', $output );
-		$this->assertStringContainsString( '30', $output );
+		$mockFile = tempnam( sys_get_temp_dir(), 'temp' );
+		$resource = fopen( $mockFile, 'wb' );
+
+		try {
+			\cli\Streams::setStream( 'out', $resource );
+			
+			$table    = new cli\Table();
+			$renderer = new cli\Table\Tabular();
+			$table->setRenderer( $renderer );
+			$table->setHeaders( array( 'Name', 'Age' ) );
+			
+			// Display a single row
+			$table->displayRow( array( 'Alice', '30' ) );
+			
+			$output = file_get_contents( $mockFile );
+			
+			// Should contain the row data with tabs
+			$this->assertStringContainsString( 'Alice', $output );
+			$this->assertStringContainsString( '30', $output );
+		} finally {
+			if ( $mockFile && file_exists( $mockFile ) ) {
+				unlink( $mockFile );
+			}
+		}
 	}
 }