-
Notifications
You must be signed in to change notification settings - Fork 26
feat: Switching to interactive when running "fab" #109
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
base: main
Are you sure you want to change the base?
Conversation
| # Catch SystemExit raised by ArgumentParser and prevent exiting | ||
| # This handles cases like --help or invalid arguments in interactive mode | ||
| if e.code != 0: | ||
| utils_ui.print(f"Command failed with exit code: {e.code}") |
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.
not sure what value it brings. users should interact with messages and text in interactive mode.
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.
WDYM?
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.
we will get here if for example we pass wrong flag... so we print this - maybe we can skip it
| except KeyboardInterrupt: | ||
| # Handle Ctrl+C gracefully during command input | ||
| utils_ui.print("\nUse 'quit' or 'exit' to leave interactive mode.") | ||
| continue | ||
| except Exception as e: | ||
| # Handle unexpected errors during prompt processing | ||
| utils_ui.print(f"Error in interactive session: {str(e)}") | ||
| utils_ui.print("Session will continue. Type 'quit' to exit safely.") | ||
| continue |
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.
can you explain why we need these changes?
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.
today when user hit Ctrl+C we exit the interactive mode.
with this change we will exit only on quit/exit same as other platform.
regarding the second exception, these lines handle exceptions in the interactive session loop, they catch errors that occur during: Context retrieval, Prompt rendering, Session management issues
These errors shouldn't terminate the interactive session. The continue statement keeps the session alive, which is the correct behavior for interactive mode (roo :)).
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.
What will happen if user types ctrl+c? It should interrupt the current running command. There are other Python REPL keyboard shortcuts that should terminate the interactive mode - CTRL+D for example, will it work?
| else: | ||
| # Display help if "fab" | ||
| fab_ui.display_help(COMMANDS) | ||
| # AUTO-REPL: When no command is provided, automatically enter interactive mode |
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.
Did you consider using a dedicated command? for example, in azure-cli the command is "az interactive". Not sure if this was already discussed and agreed.
| commands_execs += 1 | ||
| if index != len(args.command) - 1: | ||
| fab_ui.print_grey("------------------------------") | ||
| if command_parts: # Ensure we have valid command parts |
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.
why is this required? Isn't "if args.command" above sufficient?
| fab_ui.print_output_error( | ||
| FabricCLIError(err.args[0], fab_constant.ERROR_UNEXPECTED_ERROR), | ||
| output_format_type=args.output_format, | ||
| FabricCLIError( |
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.
Can we move it inside start_interactive_mode? just the printing and re-throw. I guess we would like to print it in all places where we call start_interactive_mode
📥 Pull Request
✨ Description of new changes
🎯 Overview
This PR implements automatic REPL (Read-Eval-Print-Loop) functionality by making the Fabric CLI automatically enter interactive mode when users run the fab command without arguments, eliminating the need for manual mode configuration.
🚀 Key Changes
Auto-Interactive Mode Entry
Add logic to detect when fab is run without arguments and automatically launch interactive mode
Enhanced Error Handling: Refactored main error handling with dedicated functions _handle_keyboard_interrupt() and _handle_unexpected_error() for better separation of concerns
Graceful Startup: Interactive mode now starts automatically with proper exception handling and user feedback
Interactive Mode Improvements
Enhanced interactive CLI with better command handling:
Added specific handling for fab command within interactive mode (shows helpful message instead of error)
Improved empty input handling to gracefully continue session
Better error handling for SystemExit and unexpected exceptions in command execution
Enhanced keyboard interrupt (Ctrl+C) and EOF handling with informative messages
More user-friendly error messages for invalid commands
Mode Configuration Deprecation
Added deprecation warnings for mode configuration:
Shows warning that mode configuration is deprecated when users try to set interactive/command-line mode
Maintains backward compatibility while guiding users toward the new auto-interactive behavior
Improved messaging for mode transitions
Constants and Configuration
Removed "fab" from interactive help commands list to prevent conflicts with the new auto-interactive functionality
🔄 User Experience Improvements
Simplified Workflow: Users can now simply run fab to enter interactive mode
Better Error Messages: More helpful guidance when commands fail or are invalid in interactive mode
Graceful Interruption Handling: Improved Ctrl+C behavior that doesn't exit the session immediately
Backward Compatibility: Existing mode configuration still works with deprecation guidance
🎨 Technical Implementation
Auto-Detection Logic: Main entry point detects when no command is provided and launches interactive mode
Error Isolation: Separated error handling functions for cleaner main() function
Enhanced Interactive Loop: Better command parsing and error recovery in interactive sessions
Singleton Pattern: Maintains proper InteractiveCLI singleton behavior
✅ Breaking Changes
None - this is a backward-compatible enhancement that adds functionality while maintaining existing behavior.