Issue/65 show sync messages #141
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
November 18, 2025 16:01
Previously, pdd sync hid all interactive prompts and error messages because the animation used screen=True mode which created an alternate screen that captured all output. This made debugging difficult as users could not see what was happening during operations. Changes: - Added output capture using io.StringIO and redirect_stdout/stderr - Created _display_operation_output() function to show captured output - Wrapped all 7 operations (auto-deps, generate, example, crash, verify, test, fix, update) with output capture and display - Animation temporarily pauses to show output, then resumes - Output displayed in formatted Rich panels (green for STDOUT, red for STDERR) - Clear operation headers and footers for better readability Benefits: - Users can now see detailed command output during sync - Error messages are visible for debugging - Interactive prompts are no longer hidden - Separate sections for STDOUT and STDERR when present - Animation still provides nice visual progress feedback Fixes promptdriven#65
Long file paths and output were breaking panel layout with expand=False. Changed to expand=True so panels use full terminal width and wrap text properly for better readability.
Removed Rich Panel wrapper for stdout/stderr display to avoid nested formatting conflicts. The output now uses simple headers with raw content printing, which: - Avoids broken nested box characters - Preserves original Rich formatting from operations - Provides cleaner, more readable output - Reduces pause time from 1.0s to 0.5s for better UX Also removed unused Panel import.
…tdriven#65) - Add unit tests for _display_operation_output function * Test basic stdout/stderr display functionality * Test quiet flag is respected * Test empty output is handled gracefully - Add integration tests for output capture * Test output capture works during operations * Test operation success detection is preserved * Test quiet mode suppresses output throughout workflow * Test output is displayed even when operations fail All tests pass successfully (30 tests in test_sync_orchestration.py)
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements output capture and display functionality for PDD sync operations to address issue #65. The changes enable users to see stdout/stderr from operations like generate, test, and fix, which were previously hidden during the animated sync process.
Key Changes:
- Added
_display_operation_output()function to pause animation and display captured output with formatted headers - Wrapped all 8 sync operations (auto-deps, generate, example, crash, verify, test, fix, update) with stdout/stderr capture using
io.StringIOand context managers - Output display respects the
--quietflag and resumes animation after showing results
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pdd/sync_orchestration.py | Added output capture infrastructure and wrapped all sync operations with stdout/stderr redirection |
| tests/test_sync_orchestration.py | Added 7 comprehensive tests covering basic functionality, quiet mode, empty output, integration scenarios, and failure cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Signal animation to pause | ||
| stop_event.set() | ||
| time.sleep(0.3) # Give animation time to clean up |
There was a problem hiding this comment.
The magic number 0.3 should be documented or extracted to a constant explaining why this specific duration is needed for animation cleanup.
| console.print(f"{'='*80}\n") | ||
|
|
||
| # Small pause so user can see the output | ||
| time.sleep(0.5) # Reduced from 1.0s |
There was a problem hiding this comment.
The comment 'Reduced from 1.0s' implies this was changed but doesn't explain why 0.5s is the appropriate value. Consider documenting the rationale or extracting to a named constant.
rudi-cilibrasi
approved these changes
Nov 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #65
Changes
redirect_stdout/redirect_stderr_display_operation_output()to display captured output after each operation--quietflag to suppress output when neededImplementation Details
io.StringIO()andcontextlib.redirect_stdout/stderrstop_eventTesting
test_sync_orchestration.pypytest --covfor coverage verification