-
Notifications
You must be signed in to change notification settings - Fork 199
5.0.0 #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…re checking file extension for .txt output
Add structured text output for AI model context
Use an alternate method of reading column details if the old way fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This release adds support for text-based ER diagram outputs alongside images, implements a structured Markdown representation for .txt
files, and updates testing, documentation, and dependency requirements to align with these features.
- Introduce
--text-output
flag and handle.txt
extension for structured text output in the CLI command - Implement
generateStructuredTextRepresentation
inGraphBuilder
and extend schema column retrieval - Add new PHPUnit tests for both raw DOT text and structured Markdown outputs, update README, CI matrix, and bump PHP/DBAL versions
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/GenerationTest.php | Added tests for raw text output, .txt structured output, and snapshot test |
src/GraphBuilder.php | New generateStructuredTextRepresentation method and enhanced column fetching logic |
src/GenerateDiagramCommand.php | Added --text-output option, .txt extension handling, and helper for text filenames |
composer.json | Bumped PHP requirement to ^8.2 and tightened Doctrine DBAL versions |
README.md | Documented new text and structured Markdown output options |
.github/workflows/tests.yml | Expanded CI matrix to include PHP 8.2–8.4 and Laravel 10–12 |
Comments suppressed due to low confidence (2)
composer.json:19
- Bumping PHP support to ^8.2 removes compatibility with earlier 8.x and 7.x versions. This is a breaking change; please update your changelog or release notes to inform consumers.
"php": "^8.2",
src/GraphBuilder.php:116
- Laravel's Schema facade does not provide a
getColumns
method. Consider usingSchema::getColumnListing
and then retrieving type info via the Doctrine schema manager or another supported API.
return Schema::getColumns($model->getTable());
if (is_object($column)) { | ||
$name = $column->getName(); | ||
$typeName = $column->getType()->getName(); | ||
} else { // it's an array! | ||
$name = $column['name'] ?? ''; | ||
$typeName = $column['type_name'] ?? ''; | ||
} |
Copilot
AI
Jun 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Handling two different column representations (object vs array) here adds complexity. It might be cleaner to normalize the column data in getTableColumnsFromModel
so downstream code only deals with a single, consistent structure.
if (is_object($column)) { | |
$name = $column->getName(); | |
$typeName = $column->getType()->getName(); | |
} else { // it's an array! | |
$name = $column['name'] ?? ''; | |
$typeName = $column['type_name'] ?? ''; | |
} | |
$name = $column['name']; | |
$typeName = $column['type_name']; |
Copilot uses AI. Check for mistakes.
// First check for text-output option | ||
if ($this->option('text-output') || $this->option('format') === self::FORMAT_TEXT) { | ||
$textOutput = $graph->__toString(); | ||
|
||
// If text-output option is set, write to file | ||
if ($this->option('text-output')) { | ||
$outputFileName = $this->getTextOutputFileName(); | ||
file_put_contents($outputFileName, $textOutput); | ||
$this->info(PHP_EOL); | ||
$this->info('Wrote text diagram to ' . $outputFileName); | ||
return; | ||
} | ||
|
||
// Otherwise just output to console | ||
$this->info($textOutput); | ||
return; | ||
} | ||
|
||
$graph->export($this->option('format'), $this->getOutputFileName()); | ||
// Then check for .txt extension in filename | ||
$outputFileName = $this->getOutputFileName(); | ||
if (pathinfo($outputFileName, PATHINFO_EXTENSION) === 'txt') { | ||
// Generate structured text output for .txt files | ||
$textOutput = $this->graphBuilder->generateStructuredTextRepresentation($models); | ||
file_put_contents($outputFileName, $textOutput); | ||
$this->info(PHP_EOL); | ||
$this->info('Wrote structured ER diagram to ' . $outputFileName); |
Copilot
AI
Jun 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The command handler has multiple return points and duplicated file-write logic for text and structured outputs. Consider refactoring into smaller methods (e.g., handleTextOutput
& handleStructuredOutput
) to improve readability and reduce duplication.
// First check for text-output option | |
if ($this->option('text-output') || $this->option('format') === self::FORMAT_TEXT) { | |
$textOutput = $graph->__toString(); | |
// If text-output option is set, write to file | |
if ($this->option('text-output')) { | |
$outputFileName = $this->getTextOutputFileName(); | |
file_put_contents($outputFileName, $textOutput); | |
$this->info(PHP_EOL); | |
$this->info('Wrote text diagram to ' . $outputFileName); | |
return; | |
} | |
// Otherwise just output to console | |
$this->info($textOutput); | |
return; | |
} | |
$graph->export($this->option('format'), $this->getOutputFileName()); | |
// Then check for .txt extension in filename | |
$outputFileName = $this->getOutputFileName(); | |
if (pathinfo($outputFileName, PATHINFO_EXTENSION) === 'txt') { | |
// Generate structured text output for .txt files | |
$textOutput = $this->graphBuilder->generateStructuredTextRepresentation($models); | |
file_put_contents($outputFileName, $textOutput); | |
$this->info(PHP_EOL); | |
$this->info('Wrote structured ER diagram to ' . $outputFileName); | |
// Handle text output if the option is set or format is 'text' | |
if ($this->option('text-output') || $this->option('format') === self::FORMAT_TEXT) { | |
$this->handleTextOutput($graph); | |
return; | |
} | |
// Handle structured output for .txt files | |
$outputFileName = $this->getOutputFileName(); | |
if (pathinfo($outputFileName, PATHINFO_EXTENSION) === 'txt') { | |
$this->handleStructuredOutput($models, $outputFileName); |
Copilot uses AI. Check for mistakes.
…s more robustly, accommodating both object and array formats for column data.
No description provided.