-
Notifications
You must be signed in to change notification settings - Fork 567
in app terminal logger #890
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
in app terminal logger #890
Conversation
…al page to terminal provider
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 PR adds a comprehensive logging system to APIDash that captures all outbound network traffic and JavaScript runtime logs. The implementation provides an in-app terminal view for developers to monitor API requests, JavaScript console output, and system events in real-time.
- Introduces a new Terminal/Logs page accessible from both desktop navigation rail and mobile bottom navigation
- Migrates JavaScript runtime management from service-based to provider-based architecture using Riverpod
- Implements comprehensive logging for network requests, JavaScript execution, and system events
Reviewed Changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
lib/screens/terminal/terminal_page.dart |
Main terminal interface with search, filtering, and log display |
lib/providers/terminal_providers.dart |
Core terminal state management and logging functionality |
lib/providers/js_runtime_notifier.dart |
Refactored JS runtime as Riverpod provider with terminal integration |
lib/widgets/terminal_tiles.dart |
UI components for rendering different log entry types |
lib/models/terminal/ |
Data models for terminal entries and log types |
Various navigation files | Added terminal page to desktop and mobile navigation |
Test files | Comprehensive test coverage for new terminal functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
}); | ||
|
||
group('Pre-request Script - Request Modification Tests', () { | ||
test('should modify headers correctly', () async { |
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] Empty function body with no documentation. Consider adding a comment explaining why this mock function is intentionally empty or implement minimal tracking if needed for test validation.
test('should modify headers correctly', () async { | |
test('should modify headers correctly', () async { | |
// Mock function for environment update; intentionally left empty as no update logic is needed for this test. |
Copilot uses AI. Check for mistakes.
Replaced print statements with debugPrint in tests for better Flutter logging practices. In js_runtime_notifier.dart, replaced print statements with structured logging via terminalStateProvider, providing more consistent and contextual log handling. Minor code style improvements were also made.
Corrects the navigation indices for Terminal (Logs) and Settings pages in both desktop and mobile dashboards and navbars, ensuring consistent navigation and page rendering.
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
Copilot reviewed 44 out of 49 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
lib/providers/js_runtime_notifier.dart:1
- Empty function body with misleading comment. Either implement the mock function with appropriate behavior for testing or add a comment explaining why it's intentionally empty.
import 'dart:convert';
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
LGTM 🚀
PR Description
Adds a logging screen to APIDash to capture all outbound network traffic and in app javascript logs
Related Issues
Checklist
main
branch before making this PRflutter upgrade
and verify)flutter test
) and all tests are passingAdded/updated tests?
OS on which you have developed and tested the feature?