فهرست منبع

feat(compose): add --var and --var-file support to show command (#1421)

- Add --var and --var-file options to show command
- Apply same variable precedence as generate command
- Update CHANGELOG.md with feature description
- Users can now preview variable overrides before generating files
xcad 4 ماه پیش
والد
کامیت
d96ffb417a

+ 1 - 0
CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 - Variable file support with `--var-file` flag (#1331) - Load variables from YAML file for non-interactive deployments
+- Variable override support for `show` command with `--var` and `--var-file` flags (#1421) - Preview variable overrides before generating
 
 ### Changed
 - Removed Jinja2 `| default()` filter extraction and merging (#1410) - All defaults must now be defined in template/module specs

+ 76 - 54
cli/__main__.py

@@ -103,6 +103,75 @@ def main(
         sys.exit(0)
 
 
+def _import_modules(modules_path: Path, logger: logging.Logger) -> list[str]:
+    """Import all modules and return list of failures."""
+    failed_imports = []
+    for _finder, name, ispkg in pkgutil.iter_modules([str(modules_path)]):
+        if not name.startswith("_") and name != "base":
+            try:
+                logger.debug(
+                    f"Importing module: {name} ({'package' if ispkg else 'file'})"
+                )
+                importlib.import_module(f"cli.modules.{name}")
+            except ImportError as e:
+                error_info = f"Import failed for '{name}': {e!s}"
+                failed_imports.append(error_info)
+                logger.warning(error_info)
+            except Exception as e:
+                error_info = f"Unexpected error importing '{name}': {e!s}"
+                failed_imports.append(error_info)
+                logger.error(error_info)
+    return failed_imports
+
+
+def _register_repo_command(logger: logging.Logger) -> list[str]:
+    """Register repo command and return list of failures."""
+    failed = []
+    try:
+        logger.debug("Registering repo command")
+        repo.register_cli(app)
+    except Exception as e:
+        error_info = f"Repo command registration failed: {e!s}"
+        failed.append(error_info)
+        logger.warning(error_info)
+    return failed
+
+
+def _register_module_classes(logger: logging.Logger) -> tuple[list, list[str]]:
+    """Register template-based modules and return (module_classes, failures)."""
+    failed_registrations = []
+    module_classes = list(registry.iter_module_classes())
+    logger.debug(f"Registering {len(module_classes)} template-based modules")
+
+    for _name, module_cls in module_classes:
+        try:
+            logger.debug(f"Registering module class: {module_cls.__name__}")
+            module_cls.register_cli(app)
+        except Exception as e:
+            error_info = f"Registration failed for '{module_cls.__name__}': {e!s}"
+            failed_registrations.append(error_info)
+            logger.warning(error_info)
+            console.print(f"[yellow]Warning:[/yellow] {error_info}")
+
+    return module_classes, failed_registrations
+
+
+def _build_error_details(
+    failed_imports: list[str], failed_registrations: list[str]
+) -> str:
+    """Build detailed error message from failures."""
+    error_details = []
+    if failed_imports:
+        error_details.extend(
+            ["Import failures:"] + [f"  - {err}" for err in failed_imports]
+        )
+    if failed_registrations:
+        error_details.extend(
+            ["Registration failures:"] + [f"  - {err}" for err in failed_registrations]
+        )
+    return "\n".join(error_details) if error_details else ""
+
+
 def init_app() -> None:
     """Initialize the application by discovering and registering modules.
 
@@ -111,56 +180,21 @@ def init_app() -> None:
         RuntimeError: If application initialization fails
     """
     logger = logging.getLogger(__name__)
-    failed_imports = []
-    failed_registrations = []
 
     try:
         # Auto-discover and import all modules
         modules_path = Path(cli.modules.__file__).parent
         logger.debug(f"Discovering modules in {modules_path}")
-
-        for _finder, name, ispkg in pkgutil.iter_modules([str(modules_path)]):
-            # Import both module files and packages (for multi-schema modules)
-            if not name.startswith("_") and name != "base":
-                try:
-                    logger.debug(
-                        f"Importing module: {name} ({'package' if ispkg else 'file'})"
-                    )
-                    importlib.import_module(f"cli.modules.{name}")
-                except ImportError as e:
-                    error_info = f"Import failed for '{name}': {e!s}"
-                    failed_imports.append(error_info)
-                    logger.warning(error_info)
-                except Exception as e:
-                    error_info = f"Unexpected error importing '{name}': {e!s}"
-                    failed_imports.append(error_info)
-                    logger.error(error_info)
+        failed_imports = _import_modules(modules_path, logger)
 
         # Register core repo command
-        try:
-            logger.debug("Registering repo command")
-            repo.register_cli(app)
-        except Exception as e:
-            error_info = f"Repo command registration failed: {e!s}"
-            failed_registrations.append(error_info)
-            logger.warning(error_info)
-
-        # Register template-based modules with app
-        module_classes = list(registry.iter_module_classes())
-        logger.debug(f"Registering {len(module_classes)} template-based modules")
+        repo_failures = _register_repo_command(logger)
 
-        for _name, module_cls in module_classes:
-            try:
-                logger.debug(f"Registering module class: {module_cls.__name__}")
-                module_cls.register_cli(app)
-            except Exception as e:
-                error_info = f"Registration failed for '{module_cls.__name__}': {e!s}"
-                failed_registrations.append(error_info)
-                # Log warning but don't raise exception for individual module failures
-                logger.warning(error_info)
-                console.print(f"[yellow]Warning:[/yellow] {error_info}")
+        # Register template-based modules
+        module_classes, failed_registrations = _register_module_classes(logger)
+        failed_registrations.extend(repo_failures)
 
-        # If we have no modules registered at all, that's a critical error
+        # Validate we have modules
         if not module_classes and not failed_imports:
             raise RuntimeError("No modules found to register")
 
@@ -169,25 +203,13 @@ def init_app() -> None:
         logger.info(
             f"Application initialized: {successful_modules} modules registered successfully"
         )
-
         if failed_imports:
             logger.info(f"Module import failures: {len(failed_imports)}")
         if failed_registrations:
             logger.info(f"Module registration failures: {len(failed_registrations)}")
 
     except Exception as e:
-        error_details = []
-        if failed_imports:
-            error_details.extend(
-                ["Import failures:"] + [f"  - {err}" for err in failed_imports]
-            )
-        if failed_registrations:
-            error_details.extend(
-                ["Registration failures:"]
-                + [f"  - {err}" for err in failed_registrations]
-            )
-
-        details = "\n".join(error_details) if error_details else str(e)
+        details = _build_error_details(failed_imports, failed_registrations) or str(e)
         raise RuntimeError(f"Application initialization failed: {details}") from e
 
 

+ 12 - 6
cli/core/config/config_manager.py

@@ -13,7 +13,6 @@ from ..exceptions import ConfigError, ConfigValidationError, YAMLParseError
 logger = logging.getLogger(__name__)
 
 
-
 class ConfigManager:
     """Manages configuration for the CLI application."""
 
@@ -110,7 +109,6 @@ class ConfigManager:
         except Exception as e:
             logger.warning(f"Config migration failed: {e}")
 
-
     def _read_config(self) -> dict[str, Any]:
         """Read configuration from file.
 
@@ -244,13 +242,19 @@ class ConfigManager:
 
         for i, library in enumerate(config["libraries"]):
             if not isinstance(library, dict):
-                raise ConfigValidationError(f"Library at index {i} must be a dictionary")
+                raise ConfigValidationError(
+                    f"Library at index {i} must be a dictionary"
+                )
 
             if "name" not in library:
-                raise ConfigValidationError(f"Library at index {i} missing required field 'name'")
+                raise ConfigValidationError(
+                    f"Library at index {i} missing required field 'name'"
+                )
 
             lib_type = library.get("type", "git")
-            if lib_type == "git" and ("url" not in library or "directory" not in library):
+            if lib_type == "git" and (
+                "url" not in library or "directory" not in library
+            ):
                 raise ConfigValidationError(
                     f"Git library at index {i} missing required fields 'url' and/or 'directory'"
                 )
@@ -478,7 +482,9 @@ class ConfigManager:
         # Type-specific validation
         if library_type == "git":
             if not url or not directory:
-                raise ConfigValidationError("Git libraries require 'url' and 'directory' parameters")
+                raise ConfigValidationError(
+                    "Git libraries require 'url' and 'directory' parameters"
+                )
 
             library_config = {
                 "name": name,

+ 67 - 52
cli/core/display/status_display.py

@@ -205,6 +205,67 @@ class StatusDisplayManager:
 
         return Confirm.ask("Continue?", default=default)
 
+    def _display_error_header(self, icon: str, context: str | None) -> None:
+        """Display error header with optional context."""
+        if context:
+            console_err.print(
+                f"\n[red bold]{icon} Template Rendering Error[/red bold] [dim]({context})[/dim]"
+            )
+        else:
+            console_err.print(f"\n[red bold]{icon} Template Rendering Error[/red bold]")
+        console_err.print()
+
+    def _display_error_location(self, error: TemplateRenderError) -> None:
+        """Display error file path and location."""
+        if not error.file_path:
+            return
+
+        console_err.print(f"[red]Error in file:[/red] [cyan]{error.file_path}[/cyan]")
+        if error.line_number:
+            location = f"Line {error.line_number}"
+            if error.column:
+                location += f", Column {error.column}"
+            console_err.print(f"[red]Location:[/red] {location}")
+
+    def _display_code_context(self, error: TemplateRenderError) -> None:
+        """Display code context with syntax highlighting."""
+        if not error.context_lines:
+            return
+
+        console_err.print("[bold cyan]Code Context:[/bold cyan]")
+        context_text = "\n".join(error.context_lines)
+
+        # Determine lexer for syntax highlighting
+        lexer = self._get_lexer_for_file(error.file_path)
+
+        # Try to display with syntax highlighting, fallback to plain on error
+        try:
+            self._display_syntax_panel(context_text, lexer)
+        except Exception:
+            console_err.print(Panel(context_text, border_style="red", padding=(1, 2)))
+
+        console_err.print()
+
+    def _get_lexer_for_file(self, file_path: str | None) -> str | None:
+        """Determine lexer based on file extension."""
+        if not file_path:
+            return None
+
+        file_ext = Path(file_path).suffix
+        if file_ext == ".j2":
+            base_name = Path(file_path).stem
+            base_ext = Path(base_name).suffix
+            return "jinja2" if not base_ext else None
+        return None
+
+    def _display_syntax_panel(self, text: str, lexer: str | None) -> None:
+        """Display text in a panel with optional syntax highlighting."""
+        if lexer:
+            syntax = Syntax(text, lexer, line_numbers=False, theme="monokai")
+            console_err.print(Panel(syntax, border_style="red", padding=(1, 2)))
+        else:
+            console_err.print(Panel(text, border_style="red", padding=(1, 2)))
+
     def display_template_render_error(
         self, error: TemplateRenderError, context: str | None = None
     ) -> None:
@@ -214,67 +275,21 @@ class StatusDisplayManager:
             error: TemplateRenderError exception with detailed error information
             context: Optional context information (e.g., template ID)
         """
-        # Always display errors to stderr
+        # Display error header
         icon = IconManager.get_status_icon("error")
-        if context:
-            console_err.print(
-                f"\n[red bold]{icon} Template Rendering Error[/red bold] [dim]({context})[/dim]"
-            )
-        else:
-            console_err.print(f"\n[red bold]{icon} Template Rendering Error[/red bold]")
+        self._display_error_header(icon, context)
 
-        console_err.print()
+        # Display error location
+        self._display_error_location(error)
 
         # Display error message
-        if error.file_path:
-            console_err.print(
-                f"[red]Error in file:[/red] [cyan]{error.file_path}[/cyan]"
-            )
-            if error.line_number:
-                location = f"Line {error.line_number}"
-                if error.column:
-                    location += f", Column {error.column}"
-                console_err.print(f"[red]Location:[/red] {location}")
-
         console_err.print(
             f"[red]Message:[/red] {str(error.original_error) if error.original_error else str(error)}"
         )
         console_err.print()
 
-        # Display code context if available
-        if error.context_lines:
-            console_err.print("[bold cyan]Code Context:[/bold cyan]")
-
-            # Build the context text
-            context_text = "\n".join(error.context_lines)
-
-            # Display in a panel with syntax highlighting if possible
-            file_ext = Path(error.file_path).suffix if error.file_path else ""
-            if file_ext == ".j2":
-                # Remove .j2 to get base extension for syntax highlighting
-                base_name = Path(error.file_path).stem
-                base_ext = Path(base_name).suffix
-                lexer = "jinja2" if not base_ext else None
-            else:
-                lexer = None
-
-            try:
-                if lexer:
-                    syntax = Syntax(
-                        context_text, lexer, line_numbers=False, theme="monokai"
-                    )
-                    console_err.print(Panel(syntax, border_style="red", padding=(1, 2)))
-                else:
-                    console_err.print(
-                        Panel(context_text, border_style="red", padding=(1, 2))
-                    )
-            except Exception:
-                # Fallback to plain panel if syntax highlighting fails
-                console_err.print(
-                    Panel(context_text, border_style="red", padding=(1, 2))
-                )
-
-            console_err.print()
+        # Display code context
+        self._display_code_context(error)
 
         # Display suggestions if available
         if error.suggestions:

+ 80 - 57
cli/core/display/table_display.py

@@ -149,6 +149,79 @@ class TableDisplayManager:
 
         self.parent._print_table(table)
 
+    def _build_section_label(
+        self,
+        section_name: str,
+        section_data: dict,
+        show_all: bool,
+    ) -> str:
+        """Build section label with metadata."""
+        section_desc = section_data.get("description", "")
+        section_required = section_data.get("required", False)
+        section_toggle = section_data.get("toggle")
+        section_needs = section_data.get("needs")
+
+        label = f"[cyan]{section_name}[/cyan]"
+        if section_required:
+            label += " [yellow](required)[/yellow]"
+        if section_toggle:
+            label += f" [dim](toggle: {section_toggle})[/dim]"
+        if section_needs:
+            needs_str = (
+                ", ".join(section_needs)
+                if isinstance(section_needs, list)
+                else section_needs
+            )
+            label += f" [dim](needs: {needs_str})[/dim]"
+        if show_all and section_desc:
+            label += f"\n  [dim]{section_desc}[/dim]"
+
+        return label
+
+    def _build_variable_label(
+        self,
+        var_name: str,
+        var_data: dict,
+        show_all: bool,
+    ) -> str:
+        """Build variable label with type and default value."""
+        var_type = var_data.get("type", "string")
+        var_default = var_data.get("default", "")
+        var_desc = var_data.get("description", "")
+        var_sensitive = var_data.get("sensitive", False)
+
+        label = f"[green]{var_name}[/green] [dim]({var_type})[/dim]"
+
+        if var_default is not None and var_default != "":
+            settings = self.parent.settings
+            display_val = settings.SENSITIVE_MASK if var_sensitive else str(var_default)
+            if not var_sensitive:
+                display_val = self.parent._truncate_value(
+                    display_val, settings.VALUE_MAX_LENGTH_DEFAULT
+                )
+            label += (
+                f" = [{settings.COLOR_WARNING}]{display_val}[/{settings.COLOR_WARNING}]"
+            )
+
+        if show_all and var_desc:
+            label += f"\n    [dim]{var_desc}[/dim]"
+
+        return label
+
+    def _add_section_variables(
+        self, section_node, section_vars: dict, show_all: bool
+    ) -> None:
+        """Add variables to a section node."""
+        for var_name, var_data in section_vars.items():
+            if isinstance(var_data, dict):
+                var_label = self._build_variable_label(var_name, var_data, show_all)
+                section_node.add(var_label)
+            else:
+                # Simple key-value pair
+                section_node.add(
+                    f"[green]{var_name}[/green] = [yellow]{var_data}[/yellow]"
+                )
+
     def render_config_tree(
         self, spec: dict, module_name: str, show_all: bool = False
     ) -> None:
@@ -174,65 +247,15 @@ class TableDisplayManager:
             if not isinstance(section_data, dict):
                 continue
 
-            # Determine if this is a section with variables
-            section_vars = section_data.get("vars") or {}
-            section_desc = section_data.get("description", "")
-            section_required = section_data.get("required", False)
-            section_toggle = section_data.get("toggle", None)
-            section_needs = section_data.get("needs", None)
-
-            # Build section label
-            section_label = f"[cyan]{section_name}[/cyan]"
-            if section_required:
-                section_label += " [yellow](required)[/yellow]"
-            if section_toggle:
-                section_label += f" [dim](toggle: {section_toggle})[/dim]"
-            if section_needs:
-                needs_str = (
-                    ", ".join(section_needs)
-                    if isinstance(section_needs, list)
-                    else section_needs
-                )
-                section_label += f" [dim](needs: {needs_str})[/dim]"
-
-            if show_all and section_desc:
-                section_label += f"\n  [dim]{section_desc}[/dim]"
-
+            # Build and add section
+            section_label = self._build_section_label(
+                section_name, section_data, show_all
+            )
             section_node = tree.add(section_label)
 
-            # Add variables
+            # Add variables to section
+            section_vars = section_data.get("vars") or {}
             if section_vars:
-                for var_name, var_data in section_vars.items():
-                    if isinstance(var_data, dict):
-                        var_type = var_data.get("type", "string")
-                        var_default = var_data.get("default", "")
-                        var_desc = var_data.get("description", "")
-                        var_sensitive = var_data.get("sensitive", False)
-
-                        # Build variable label
-                        var_label = f"[green]{var_name}[/green] [dim]({var_type})[/dim]"
-
-                        if var_default is not None and var_default != "":
-                            settings = self.parent.settings
-                            display_val = (
-                                settings.SENSITIVE_MASK
-                                if var_sensitive
-                                else str(var_default)
-                            )
-                            if not var_sensitive:
-                                display_val = self.parent._truncate_value(
-                                    display_val, settings.VALUE_MAX_LENGTH_DEFAULT
-                                )
-                            var_label += f" = [{settings.COLOR_WARNING}]{display_val}[/{settings.COLOR_WARNING}]"
-
-                        if show_all and var_desc:
-                            var_label += f"\n    [dim]{var_desc}[/dim]"
-
-                        section_node.add(var_label)
-                    else:
-                        # Simple key-value pair
-                        section_node.add(
-                            f"[green]{var_name}[/green] = [yellow]{var_data}[/yellow]"
-                        )
+                self._add_section_variables(section_node, section_vars, show_all)
 
         self.parent._print_tree(tree)

+ 2 - 1
cli/core/input/__init__.py

@@ -6,5 +6,6 @@ and validation across the entire CLI application.
 
 from .input_manager import InputManager
 from .input_settings import InputSettings
+from .prompt_manager import PromptHandler
 
-__all__ = ["InputManager", "InputSettings"]
+__all__ = ["InputManager", "InputSettings", "PromptHandler"]

+ 69 - 67
cli/core/input/prompt_manager.py

@@ -6,8 +6,8 @@ from typing import Any, Callable
 from rich.console import Console
 from rich.prompt import Confirm, IntPrompt, Prompt
 
-from .display import DisplayManager
-from .template import Variable, VariableCollection
+from ..display import DisplayManager
+from ..template import Variable, VariableCollection
 
 logger = logging.getLogger(__name__)
 
@@ -19,6 +19,67 @@ class PromptHandler:
         self.console = Console()
         self.display = DisplayManager()
 
+    def _handle_section_toggle(self, section, collected: dict[str, Any]) -> bool:
+        """Handle section toggle variable and return whether section should be enabled."""
+        if section.required:
+            logger.debug(
+                f"Processing required section '{section.key}' without toggle prompt"
+            )
+            return True
+
+        if not section.toggle:
+            return True
+
+        toggle_var = section.variables.get(section.toggle)
+        if not toggle_var:
+            return True
+
+        current_value = toggle_var.convert(toggle_var.value)
+        new_value = self._prompt_variable(toggle_var, required=section.required)
+
+        if new_value != current_value:
+            collected[toggle_var.name] = new_value
+            toggle_var.value = new_value
+
+        return section.is_enabled()
+
+    def _should_skip_variable(
+        self,
+        var_name: str,
+        section,
+        variables: VariableCollection,
+        section_enabled: bool,
+    ) -> bool:
+        """Determine if a variable should be skipped during collection."""
+        if section.toggle and var_name == section.toggle:
+            return True
+
+        if not variables.is_variable_satisfied(var_name):
+            logger.debug(f"Skipping variable '{var_name}' - needs not satisfied")
+            return True
+
+        if not section_enabled:
+            logger.debug(
+                f"Skipping variable '{var_name}' from disabled section '{section.key}'"
+            )
+            return True
+
+        return False
+
+    def _collect_variable_value(
+        self, variable: Variable, section, collected: dict[str, Any]
+    ) -> None:
+        """Collect a single variable value and update if changed."""
+        current_value = variable.convert(variable.value)
+        new_value = self._prompt_variable(variable, required=section.required)
+
+        if variable.autogenerated and new_value is None:
+            collected[variable.name] = None
+            variable.value = None
+        elif new_value != current_value:
+            collected[variable.name] = new_value
+            variable.value = new_value
+
     def collect_variables(self, variables: VariableCollection) -> dict[str, Any]:
         """Collect values for variables by iterating through sections.
 
@@ -33,82 +94,23 @@ class PromptHandler:
             return {}
 
         collected: dict[str, Any] = {}
-        prompted_variables: set[str] = (
-            set()
-        )  # Track which variables we've already prompted for
 
-        # Process each section (only satisfied dependencies, include disabled for toggle handling)
-        for section_key, section in variables.iter_active_sections(
+        for _section_key, section in variables.iter_active_sections(
             include_disabled=True
         ):
             if not section.variables:
                 continue
 
-            # Always show section header first
             self.display.display_section(section.title, section.description)
+            section_enabled = self._handle_section_toggle(section, collected)
 
-            # Track whether this section will be enabled
-            section_will_be_enabled = True
-
-            # Handle section toggle - skip for required sections
-            if section.required:
-                # Required sections are always processed, no toggle prompt needed
-                logger.debug(
-                    f"Processing required section '{section.key}' without toggle prompt"
-                )
-            elif section.toggle:
-                toggle_var = section.variables.get(section.toggle)
-                if toggle_var:
-                    # Prompt for toggle variable using standard variable prompting logic
-                    # This ensures consistent handling of description, extra text, validation hints, etc.
-                    current_value = toggle_var.convert(toggle_var.value)
-                    new_value = self._prompt_variable(
-                        toggle_var, required=section.required
-                    )
-
-                    if new_value != current_value:
-                        collected[toggle_var.name] = new_value
-                        toggle_var.value = new_value
-
-                    # Use section's native is_enabled() method
-                    if not section.is_enabled():
-                        section_will_be_enabled = False
-
-            # Collect variables in this section
             for var_name, variable in section.variables.items():
-                # Skip toggle variable (already handled)
-                if section.toggle and var_name == section.toggle:
-                    continue
-
-                # Skip variables with unsatisfied needs (similar to display logic)
-                if not variables.is_variable_satisfied(var_name):
-                    logger.debug(
-                        f"Skipping variable '{var_name}' - needs not satisfied"
-                    )
-                    continue
-
-                # Skip all variables if section is disabled
-                if not section_will_be_enabled:
-                    logger.debug(
-                        f"Skipping variable '{var_name}' from disabled section '{section_key}'"
-                    )
+                if self._should_skip_variable(
+                    var_name, section, variables, section_enabled
+                ):
                     continue
 
-                # Prompt for the variable
-                current_value = variable.convert(variable.value)
-                # Pass section.required so _prompt_variable can enforce required inputs
-                new_value = self._prompt_variable(variable, required=section.required)
-
-                # Track that we've prompted for this variable
-                prompted_variables.add(var_name)
-
-                # For autogenerated variables, always update even if None (signals autogeneration)
-                if variable.autogenerated and new_value is None:
-                    collected[var_name] = None
-                    variable.value = None
-                elif new_value != current_value:
-                    collected[var_name] = new_value
-                    variable.value = new_value
+                self._collect_variable_value(variable, section, collected)
 
         logger.info(f"Variable collection completed. Collected {len(collected)} values")
         return collected

+ 43 - 40
cli/core/library.py

@@ -159,6 +159,40 @@ class LibraryManager:
         self.config = ConfigManager()
         self.libraries = self._load_libraries_from_config()
 
+    def _resolve_git_library_path(
+        self, name: str, lib_config: dict, libraries_path: Path
+    ) -> Path:
+        """Resolve path for a git-based library."""
+        directory = lib_config.get("directory", ".")
+        library_base = libraries_path / name
+        if directory and directory != ".":
+            return library_base / directory
+        return library_base
+
+    def _resolve_static_library_path(self, name: str, lib_config: dict) -> Path | None:
+        """Resolve path for a static library."""
+        path_str = lib_config.get("path")
+        if not path_str:
+            logger.warning(f"Static library '{name}' has no path configured")
+            return None
+
+        library_path = Path(path_str).expanduser()
+        if not library_path.is_absolute():
+            library_path = (self.config.config_path.parent / library_path).resolve()
+        return library_path
+
+    def _warn_missing_library(
+        self, name: str, library_path: Path, lib_type: str
+    ) -> None:
+        """Log warning about missing library."""
+        if lib_type == "git":
+            logger.warning(
+                f"Library '{name}' not found at {library_path}. "
+                f"Run 'repo update' to sync libraries."
+            )
+        else:
+            logger.warning(f"Static library '{name}' not found at {library_path}")
+
     def _load_libraries_from_config(self) -> list[Library]:
         """Load libraries from configuration.
 
@@ -167,8 +201,6 @@ class LibraryManager:
         """
         libraries = []
         libraries_path = self.config.get_libraries_path()
-
-        # Get library configurations from config
         library_configs = self.config.get_libraries()
 
         for i, lib_config in enumerate(library_configs):
@@ -178,38 +210,17 @@ class LibraryManager:
                 continue
 
             name = lib_config.get("name")
-            lib_type = lib_config.get(
-                "type", "git"
-            )  # Default to "git" for backward compat
+            lib_type = lib_config.get("type", "git")
 
-            # Handle library type-specific path resolution
+            # Resolve library path based on type
             if lib_type == "git":
-                # Existing git logic
-                directory = lib_config.get("directory", ".")
-
-                # Build path to library: ~/.config/boilerplates/libraries/{name}/{directory}/
-                # For sparse-checkout, files remain in the specified directory
-                library_base = libraries_path / name
-                if directory and directory != ".":
-                    library_path = library_base / directory
-                else:
-                    library_path = library_base
-
+                library_path = self._resolve_git_library_path(
+                    name, lib_config, libraries_path
+                )
             elif lib_type == "static":
-                # New static logic - use path directly
-                path_str = lib_config.get("path")
-                if not path_str:
-                    logger.warning(f"Static library '{name}' has no path configured")
+                library_path = self._resolve_static_library_path(name, lib_config)
+                if not library_path:
                     continue
-
-                # Expand ~ and resolve relative paths
-                library_path = Path(path_str).expanduser()
-                if not library_path.is_absolute():
-                    # Resolve relative to config directory
-                    library_path = (
-                        self.config.config_path.parent / library_path
-                    ).resolve()
-
             else:
                 logger.warning(
                     f"Unknown library type '{lib_type}' for library '{name}'"
@@ -218,18 +229,10 @@ class LibraryManager:
 
             # Check if library path exists
             if not library_path.exists():
-                if lib_type == "git":
-                    logger.warning(
-                        f"Library '{name}' not found at {library_path}. "
-                        f"Run 'repo update' to sync libraries."
-                    )
-                else:
-                    logger.warning(
-                        f"Static library '{name}' not found at {library_path}"
-                    )
+                self._warn_missing_library(name, library_path, lib_type)
                 continue
 
-            # Create Library instance with type and priority based on order (first = highest priority)
+            # Create Library instance with priority based on order
             priority = len(library_configs) - i
             libraries.append(
                 Library(

+ 154 - 118
cli/core/module/base_commands.py

@@ -97,8 +97,10 @@ def search_templates(module_instance, query: str) -> list:
     return filtered_templates
 
 
-def show_template(module_instance, id: str) -> None:
-    """Show template details."""
+def show_template(
+    module_instance, id: str, var: list[str] | None = None, var_file: str | None = None
+) -> None:
+    """Show template details with optional variable overrides."""
     logger.debug(f"Showing template '{id}' from module '{module_instance.name}'")
     template = module_instance._load_template_by_id(id)
 
@@ -108,20 +110,14 @@ def show_template(module_instance, id: str) -> None:
         )
         return
 
-    # Apply config defaults (same as in generate)
-    # This ensures the display shows the actual defaults that will be used
+    # Apply defaults and overrides (same precedence as generate command)
     if template.variables:
         config = ConfigManager()
-        config_defaults = config.get_defaults(module_instance.name)
-
-        if config_defaults:
-            logger.debug(f"Loading config defaults for module '{module_instance.name}'")
-            # Apply config defaults (this respects the variable types and validation)
-            successful = template.variables.apply_defaults(config_defaults, "config")
-            if successful:
-                logger.debug(f"Applied config defaults for: {', '.join(successful)}")
+        apply_variable_defaults(template, config, module_instance.name)
+        apply_var_file(template, var_file, module_instance.display)
+        apply_cli_overrides(template, var)
 
-        # Re-sort sections after applying config (toggle values may have changed)
+        # Re-sort sections after applying overrides (toggle values may have changed)
         template.variables.sort_sections()
 
         # Reset disabled bool variables to False to prevent confusion
@@ -202,27 +198,10 @@ def get_generation_confirmation(
     return True
 
 
-def execute_dry_run(
-    id: str,
-    output_dir: Path,
-    rendered_files: dict[str, str],
-    show_files: bool,
-    display: DisplayManager,
-) -> None:
-    """Execute dry run mode with comprehensive simulation."""
-    display.display_info("")
-    display.display_info(
-        "[bold cyan]Dry Run Mode - Simulating File Generation[/bold cyan]"
-    )
-    display.display_info("")
-
-    # Simulate directory creation
-    display.heading("Directory Operations")
-
-    # Check if output directory exists
+def _check_directory_permissions(output_dir: Path, display: DisplayManager) -> None:
+    """Check directory existence and write permissions."""
     if output_dir.exists():
         display.display_success(f"Output directory exists: [cyan]{output_dir}[/cyan]")
-        # Check if we have write permissions
         if os.access(output_dir, os.W_OK):
             display.display_success("Write permission verified")
         else:
@@ -231,32 +210,27 @@ def execute_dry_run(
         display.display_info(
             f"  [dim]→[/dim] Would create output directory: [cyan]{output_dir}[/cyan]"
         )
-        # Check if parent directory exists and is writable
         parent = output_dir.parent
         if parent.exists() and os.access(parent, os.W_OK):
             display.display_success("Parent directory writable")
         else:
             display.display_warning("Parent directory may not be writable")
 
-    # Collect unique subdirectories that would be created
+
+def _collect_subdirectories(rendered_files: dict[str, str]) -> set[Path]:
+    """Collect unique subdirectories from file paths."""
     subdirs = set()
     for file_path in rendered_files:
         parts = Path(file_path).parts
         for i in range(1, len(parts)):
             subdirs.add(Path(*parts[:i]))
+    return subdirs
 
-    if subdirs:
-        display.display_info(
-            f"  [dim]→[/dim] Would create {len(subdirs)} subdirectory(ies)"
-        )
-        for subdir in sorted(subdirs):
-            display.display_info(f"    [dim]📁[/dim] {subdir}/")
-
-    display.display_info("")
-
-    # Display file operations in a table
-    display.heading("File Operations")
 
+def _analyze_file_operations(
+    output_dir: Path, rendered_files: dict[str, str]
+) -> tuple[list[tuple[str, int, str]], int, int, int]:
+    """Analyze file operations and return statistics."""
     total_size = 0
     new_files = 0
     overwrite_files = 0
@@ -267,7 +241,6 @@ def execute_dry_run(
         file_size = len(content.encode("utf-8"))
         total_size += file_size
 
-        # Determine status
         if full_path.exists():
             status = "Overwrite"
             overwrite_files += 1
@@ -277,17 +250,58 @@ def execute_dry_run(
 
         file_operations.append((file_path, file_size, status))
 
-    display.display_file_operation_table(file_operations)
-    display.display_info("")
+    return file_operations, total_size, new_files, overwrite_files
 
-    # Summary statistics
+
+def _format_size(total_size: int) -> str:
+    """Format byte size into human-readable string."""
     if total_size < BYTES_PER_KB:
-        size_str = f"{total_size}B"
+        return f"{total_size}B"
     elif total_size < BYTES_PER_MB:
-        size_str = f"{total_size / BYTES_PER_KB:.1f}KB"
+        return f"{total_size / BYTES_PER_KB:.1f}KB"
     else:
-        size_str = f"{total_size / BYTES_PER_MB:.1f}MB"
+        return f"{total_size / BYTES_PER_MB:.1f}MB"
 
+
+def execute_dry_run(
+    id: str,
+    output_dir: Path,
+    rendered_files: dict[str, str],
+    show_files: bool,
+    display: DisplayManager,
+) -> None:
+    """Execute dry run mode with comprehensive simulation."""
+    display.display_info("")
+    display.display_info(
+        "[bold cyan]Dry Run Mode - Simulating File Generation[/bold cyan]"
+    )
+    display.display_info("")
+
+    # Simulate directory creation
+    display.heading("Directory Operations")
+    _check_directory_permissions(output_dir, display)
+
+    # Collect and display subdirectories
+    subdirs = _collect_subdirectories(rendered_files)
+    if subdirs:
+        display.display_info(
+            f"  [dim]→[/dim] Would create {len(subdirs)} subdirectory(ies)"
+        )
+        for subdir in sorted(subdirs):
+            display.display_info(f"    [dim]📁[/dim] {subdir}/")
+
+    display.display_info("")
+
+    # Display file operations in a table
+    display.heading("File Operations")
+    file_operations, total_size, new_files, overwrite_files = _analyze_file_operations(
+        output_dir, rendered_files
+    )
+    display.display_file_operation_table(file_operations)
+    display.display_info("")
+
+    # Summary statistics
+    size_str = _format_size(total_size)
     summary_items = {
         "Total files:": str(len(rendered_files)),
         "New files:": str(new_files),
@@ -339,83 +353,94 @@ def write_generated_files(
     logger.info(f"Template written to directory: {output_dir}")
 
 
-def generate_template(
+def _prepare_template(
     module_instance,
     id: str,
-    directory: str | None,
-    interactive: bool,
-    var: list[str] | None,
     var_file: str | None,
-    dry_run: bool,
-    show_files: bool,
-    quiet: bool,
-) -> None:
-    """Generate from template."""
-    logger.info(
-        f"Starting generation for template '{id}' from module '{module_instance.name}'"
-    )
-
-    # Create a display manager with quiet mode if needed
-    display = DisplayManager(quiet=quiet) if quiet else module_instance.display
-
+    var: list[str] | None,
+    display: DisplayManager,
+):
+    """Load template and apply all defaults/overrides."""
     template = module_instance._load_template_by_id(id)
-
-    # Apply defaults and overrides (in precedence order)
     config = ConfigManager()
     apply_variable_defaults(template, config, module_instance.name)
     apply_var_file(template, var_file, display)
     apply_cli_overrides(template, var)
 
-    # Re-sort sections after all overrides (toggle values may have changed)
     if template.variables:
         template.variables.sort_sections()
-
-        # Reset disabled bool variables to False to prevent confusion
         reset_vars = template.variables.reset_disabled_bool_variables()
         if reset_vars:
             logger.debug(f"Reset {len(reset_vars)} disabled bool variables to False")
 
-    if not quiet:
-        module_instance.display.display_template(template, id)
-        module_instance.display.display_info("")
+    return template
+
 
-    # Collect variable values
+def _render_template(template, id: str, display: DisplayManager, interactive: bool):
+    """Validate, render template and collect variable values."""
     variable_values = collect_variable_values(template, interactive)
 
-    try:
-        # Validate and render template
-        if template.variables:
-            template.variables.validate_all()
+    if template.variables:
+        template.variables.validate_all()
 
-        # Check if we're in debug mode (logger level is DEBUG)
-        debug_mode = logger.isEnabledFor(logging.DEBUG)
+    debug_mode = logger.isEnabledFor(logging.DEBUG)
+    rendered_files, variable_values = template.render(
+        template.variables, debug=debug_mode
+    )
 
-        rendered_files, variable_values = template.render(
-            template.variables, debug=debug_mode
+    if not rendered_files:
+        display.display_error(
+            "Template rendering returned no files",
+            context="template generation",
         )
+        raise Exit(code=1)
 
-        if not rendered_files:
-            display.display_error(
-                "Template rendering returned no files",
-                context="template generation",
-            )
-            raise Exit(code=1)
+    logger.info(f"Successfully rendered template '{id}'")
+    return rendered_files, variable_values
 
-        logger.info(f"Successfully rendered template '{id}'")
 
-        # Determine output directory
-        if directory:
-            output_dir = Path(directory)
-            # Check if path looks like an absolute path but is missing the leading slash
-            if not output_dir.is_absolute() and str(output_dir).startswith(
-                ("Users/", "home/", "usr/", "opt/", "var/", "tmp/")
-            ):
-                output_dir = Path("/") / output_dir
-                logger.debug(
-                    f"Normalized relative-looking absolute path to: {output_dir}"
-                )
-        else:
-            output_dir = Path(id)
+def _determine_output_dir(directory: str | None, id: str) -> Path:
+    """Determine and normalize output directory path."""
+    if directory:
+        output_dir = Path(directory)
+        if not output_dir.is_absolute() and str(output_dir).startswith(
+            ("Users/", "home/", "usr/", "opt/", "var/", "tmp/")
+        ):
+            output_dir = Path("/") / output_dir
+            logger.debug(f"Normalized relative-looking absolute path to: {output_dir}")
+    else:
+        output_dir = Path(id)
+    return output_dir
+
+
+def generate_template(
+    module_instance,
+    id: str,
+    directory: str | None,
+    interactive: bool,
+    var: list[str] | None,
+    var_file: str | None,
+    dry_run: bool,
+    show_files: bool,
+    quiet: bool,
+) -> None:
+    """Generate from template."""
+    logger.info(
+        f"Starting generation for template '{id}' from module '{module_instance.name}'"
+    )
+
+    display = DisplayManager(quiet=quiet) if quiet else module_instance.display
+    template = _prepare_template(module_instance, id, var_file, var, display)
+
+    if not quiet:
+        module_instance.display.display_template(template, id)
+        module_instance.display.display_info("")
+
+    try:
+        rendered_files, variable_values = _render_template(
+            template, id, display, interactive
+        )
+        output_dir = _determine_output_dir(directory, id)
 
         # Check for conflicts and get confirmation (skip in quiet mode)
         if not quiet:
@@ -425,7 +450,6 @@ def generate_template(
             if existing_files is None:
                 return  # User cancelled
 
-            # Get final confirmation for generation
             dir_not_empty = output_dir.exists() and any(output_dir.iterdir())
             if not get_generation_confirmation(
                 output_dir,
@@ -437,9 +461,6 @@ def generate_template(
                 display,
             ):
                 return  # User cancelled
-        else:
-            # In quiet mode, just check for existing files without prompts
-            existing_files = []
 
         # Execute generation (dry run or actual)
         if dry_run:
@@ -453,7 +474,6 @@ def generate_template(
             display.display_next_steps(template.metadata.next_steps, variable_values)
 
     except TemplateRenderError as e:
-        # Display enhanced error information for template rendering errors (always show errors)
         display.display_template_render_error(e, context=f"template '{id}'")
         raise Exit(code=1) from None
     except Exception as e:
@@ -473,7 +493,9 @@ def validate_templates(
     template = _load_template_for_validation(module_instance, template_id, path)
 
     if template:
-        _validate_single_template(module_instance, template, template_id, verbose, semantic)
+        _validate_single_template(
+            module_instance, template, template_id, verbose, semantic
+        )
     else:
         _validate_all_templates(module_instance, verbose)
 
@@ -508,13 +530,17 @@ def _load_template_for_validation(module_instance, template_id: str, path: str |
             )
             return template
         except Exception as e:
-            module_instance.display.display_error(f"Failed to load template '{template_id}': {e}")
+            module_instance.display.display_error(
+                f"Failed to load template '{template_id}': {e}"
+            )
             raise Exit(code=1) from None
 
     return None
 
 
-def _validate_single_template(module_instance, template, template_id: str, verbose: bool, semantic: bool) -> None:
+def _validate_single_template(
+    module_instance, template, template_id: str, verbose: bool, semantic: bool
+) -> None:
     """Validate a single template."""
     try:
         # Jinja2 validation
@@ -540,14 +566,18 @@ def _validate_single_template(module_instance, template, template_id: str, verbo
         module_instance.display.display_info(f"\n{e}")
         raise Exit(code=1) from None
     except Exception as e:
-        module_instance.display.display_error(f"Unexpected error validating '{template_id}': {e}")
+        module_instance.display.display_error(
+            f"Unexpected error validating '{template_id}': {e}"
+        )
         raise Exit(code=1) from None
 
 
 def _run_semantic_validation(module_instance, template, verbose: bool) -> None:
     """Run semantic validation on rendered template files."""
     module_instance.display.display_info("")
-    module_instance.display.display_info("[bold cyan]Running semantic validation...[/bold cyan]")
+    module_instance.display.display_info(
+        "[bold cyan]Running semantic validation...[/bold cyan]"
+    )
 
     registry = get_validator_registry()
     debug_mode = logger.isEnabledFor(logging.DEBUG)
@@ -573,12 +603,18 @@ def _run_semantic_validation(module_instance, template, verbose: bool) -> None:
 
 def _display_validation_details(module_instance, template, semantic: bool) -> None:
     """Display verbose validation details."""
-    module_instance.display.display_info(f"\n[dim]Template path: {template.template_dir}[/dim]")
-    module_instance.display.display_info(f"[dim]Found {len(template.used_variables)} variables[/dim]")
+    module_instance.display.display_info(
+        f"\n[dim]Template path: {template.template_dir}[/dim]"
+    )
+    module_instance.display.display_info(
+        f"[dim]Found {len(template.used_variables)} variables[/dim]"
+    )
     if semantic:
         debug_mode = logger.isEnabledFor(logging.DEBUG)
         rendered_files, _ = template.render(template.variables, debug=debug_mode)
-        module_instance.display.display_info(f"[dim]Generated {len(rendered_files)} files[/dim]")
+        module_instance.display.display_info(
+            f"[dim]Generated {len(rendered_files)} files[/dim]"
+        )
 
 
 def _validate_all_templates(module_instance, verbose: bool) -> None:

+ 29 - 5
cli/core/module/base_module.py

@@ -49,7 +49,9 @@ class Module(ABC):
 
     def __init__(self) -> None:
         # Validate required class attributes
-        if not hasattr(self.__class__, 'name') or not hasattr(self.__class__, 'description'):
+        if not hasattr(self.__class__, "name") or not hasattr(
+            self.__class__, "description"
+        ):
             raise TypeError(
                 f"Module {self.__class__.__name__} must define 'name' and 'description' class attributes"
             )
@@ -162,9 +164,28 @@ class Module(ABC):
         """Search for templates by ID containing the search string."""
         return search_templates(self, query)
 
-    def show(self, id: str) -> None:
-        """Show template details."""
-        return show_template(self, id)
+    def show(
+        self,
+        id: str,
+        var: Annotated[
+            list[str] | None,
+            Option(
+                "--var",
+                "-v",
+                help="Variable override (repeatable). Supports: KEY=VALUE or KEY VALUE",
+            ),
+        ] = None,
+        var_file: Annotated[
+            str | None,
+            Option(
+                "--var-file",
+                "-f",
+                help="Load variables from YAML file (overrides config defaults)",
+            ),
+        ] = None,
+    ) -> None:
+        """Show template details with optional variable overrides."""
+        return show_template(self, id, var, var_file)
 
     def generate(
         self,
@@ -197,7 +218,10 @@ class Module(ABC):
             ),
         ] = None,
         dry_run: Annotated[
-            bool, Option("--dry-run", help="Preview template generation without writing files")
+            bool,
+            Option(
+                "--dry-run", help="Preview template generation without writing files"
+            ),
         ] = False,
         show_files: Annotated[
             bool,

+ 1 - 0
cli/core/module/config_commands.py

@@ -11,6 +11,7 @@ from ..config import ConfigManager
 
 logger = logging.getLogger(__name__)
 
+
 def config_get(module_instance, var_name: str | None = None) -> None:
     """Get default value(s) for this module."""
     config = ConfigManager()

+ 74 - 63
cli/core/prompt.py

@@ -19,6 +19,71 @@ class PromptHandler:
         self.console = Console()
         self.display = DisplayManager()
 
+    def _handle_section_toggle(self, section, collected: dict[str, Any]) -> bool:
+        """Handle section toggle variable and return whether section should be enabled."""
+        if section.required:
+            logger.debug(
+                f"Processing required section '{section.key}' without toggle prompt"
+            )
+            return True
+
+        if not section.toggle:
+            return True
+
+        toggle_var = section.variables.get(section.toggle)
+        if not toggle_var:
+            return True
+
+        current_value = toggle_var.convert(toggle_var.value)
+        new_value = self._prompt_variable(toggle_var, required=section.required)
+
+        if new_value != current_value:
+            collected[toggle_var.name] = new_value
+            toggle_var.value = new_value
+
+        return section.is_enabled()
+
+    def _should_skip_variable(
+        self,
+        var_name: str,
+        section,
+        variables: VariableCollection,
+        section_enabled: bool,
+    ) -> bool:
+        """Determine if a variable should be skipped during collection."""
+        # Skip toggle variable (already handled)
+        if section.toggle and var_name == section.toggle:
+            return True
+
+        # Skip variables with unsatisfied needs
+        if not variables.is_variable_satisfied(var_name):
+            logger.debug(f"Skipping variable '{var_name}' - needs not satisfied")
+            return True
+
+        # Skip all variables if section is disabled
+        if not section_enabled:
+            logger.debug(
+                f"Skipping variable '{var_name}' from disabled section '{section.key}'"
+            )
+            return True
+
+        return False
+
+    def _collect_variable_value(
+        self, variable: Variable, section, collected: dict[str, Any]
+    ) -> None:
+        """Collect a single variable value and update if changed."""
+        current_value = variable.convert(variable.value)
+        new_value = self._prompt_variable(variable, required=section.required)
+
+        # For autogenerated variables, always update even if None
+        if variable.autogenerated and new_value is None:
+            collected[variable.name] = None
+            variable.value = None
+        elif new_value != current_value:
+            collected[variable.name] = new_value
+            variable.value = new_value
+
     def collect_variables(self, variables: VariableCollection) -> dict[str, Any]:
         """Collect values for variables by iterating through sections.
 
@@ -33,82 +98,28 @@ class PromptHandler:
             return {}
 
         collected: dict[str, Any] = {}
-        prompted_variables: set[str] = (
-            set()
-        )  # Track which variables we've already prompted for
 
-        # Process each section (only satisfied dependencies, include disabled for toggle handling)
-        for section_key, section in variables.iter_active_sections(
+        # Process each section
+        for _section_key, section in variables.iter_active_sections(
             include_disabled=True
         ):
             if not section.variables:
                 continue
 
-            # Always show section header first
+            # Display section header
             self.display.display_section(section.title, section.description)
 
-            # Track whether this section will be enabled
-            section_will_be_enabled = True
-
-            # Handle section toggle - skip for required sections
-            if section.required:
-                # Required sections are always processed, no toggle prompt needed
-                logger.debug(
-                    f"Processing required section '{section.key}' without toggle prompt"
-                )
-            elif section.toggle:
-                toggle_var = section.variables.get(section.toggle)
-                if toggle_var:
-                    # Prompt for toggle variable using standard variable prompting logic
-                    # This ensures consistent handling of description, extra text, validation hints, etc.
-                    current_value = toggle_var.convert(toggle_var.value)
-                    new_value = self._prompt_variable(
-                        toggle_var, required=section.required
-                    )
-
-                    if new_value != current_value:
-                        collected[toggle_var.name] = new_value
-                        toggle_var.value = new_value
-
-                    # Use section's native is_enabled() method
-                    if not section.is_enabled():
-                        section_will_be_enabled = False
+            # Handle toggle and determine if section is enabled
+            section_enabled = self._handle_section_toggle(section, collected)
 
             # Collect variables in this section
             for var_name, variable in section.variables.items():
-                # Skip toggle variable (already handled)
-                if section.toggle and var_name == section.toggle:
-                    continue
-
-                # Skip variables with unsatisfied needs (similar to display logic)
-                if not variables.is_variable_satisfied(var_name):
-                    logger.debug(
-                        f"Skipping variable '{var_name}' - needs not satisfied"
-                    )
-                    continue
-
-                # Skip all variables if section is disabled
-                if not section_will_be_enabled:
-                    logger.debug(
-                        f"Skipping variable '{var_name}' from disabled section '{section_key}'"
-                    )
+                if self._should_skip_variable(
+                    var_name, section, variables, section_enabled
+                ):
                     continue
 
-                # Prompt for the variable
-                current_value = variable.convert(variable.value)
-                # Pass section.required so _prompt_variable can enforce required inputs
-                new_value = self._prompt_variable(variable, required=section.required)
-
-                # Track that we've prompted for this variable
-                prompted_variables.add(var_name)
-
-                # For autogenerated variables, always update even if None (signals autogeneration)
-                if variable.autogenerated and new_value is None:
-                    collected[var_name] = None
-                    variable.value = None
-                elif new_value != current_value:
-                    collected[var_name] = new_value
-                    variable.value = new_value
+                self._collect_variable_value(variable, section, collected)
 
         logger.info(f"Variable collection completed. Collected {len(collected)} values")
         return collected

+ 153 - 136
cli/core/repo.py

@@ -74,7 +74,9 @@ def _clone_or_pull_repo(
         return _clone_new_repo(name, url, target_path, branch, sparse_dir)
 
 
-def _pull_repo_updates(name: str, target_path: Path, branch: str | None) -> tuple[bool, str]:
+def _pull_repo_updates(
+    name: str, target_path: Path, branch: str | None
+) -> tuple[bool, str]:
     """Pull updates for an existing repository."""
     logger.debug(f"Pulling updates for library '{name}' at {target_path}")
 
@@ -114,34 +116,24 @@ def _clone_sparse_repo(
 ) -> tuple[bool, str]:
     """Clone repository with sparse-checkout."""
     logger.debug(f"Using sparse-checkout for directory: {sparse_dir}")
-
     target_path.mkdir(parents=True, exist_ok=True)
 
-    # Initialize git repo
-    success, stdout, stderr = _run_git_command(["init"], cwd=target_path)
-    if not success:
-        return False, f"Failed to initialize repo: {stderr or stdout}"
-
-    # Add remote
-    success, stdout, stderr = _run_git_command(
-        ["remote", "add", "origin", url], cwd=target_path
-    )
-    if not success:
-        return False, f"Failed to add remote: {stderr or stdout}"
-
-    # Enable sparse-checkout
-    success, stdout, stderr = _run_git_command(
-        ["sparse-checkout", "init", "--no-cone"], cwd=target_path
-    )
-    if not success:
-        return False, f"Failed to enable sparse-checkout: {stderr or stdout}"
-
-    # Set sparse-checkout directory
-    success, stdout, stderr = _run_git_command(
-        ["sparse-checkout", "set", f"{sparse_dir}/*"], cwd=target_path
-    )
-    if not success:
-        return False, f"Failed to set sparse-checkout directory: {stderr or stdout}"
+    # Define git operations to perform sequentially
+    operations = [
+        (["init"], "Failed to initialize repo"),
+        (["remote", "add", "origin", url], "Failed to add remote"),
+        (["sparse-checkout", "init", "--no-cone"], "Failed to enable sparse-checkout"),
+        (
+            ["sparse-checkout", "set", f"{sparse_dir}/*"],
+            "Failed to set sparse-checkout directory",
+        ),
+    ]
+
+    # Execute initial operations
+    for cmd, error_msg in operations:
+        success, stdout, stderr = _run_git_command(cmd, cwd=target_path)
+        if not success:
+            return False, f"{error_msg}: {stderr or stdout}"
 
     # Fetch and checkout
     fetch_branch = branch if branch else "main"
@@ -151,11 +143,17 @@ def _clone_sparse_repo(
     if not success:
         return False, f"Fetch failed: {stderr or stdout}"
 
-    success, stdout, stderr = _run_git_command(["checkout", fetch_branch], cwd=target_path)
-    if not success:
-        return False, f"Checkout failed: {stderr or stdout}"
+    success, stdout, stderr = _run_git_command(
+        ["checkout", fetch_branch], cwd=target_path
+    )
+    result_success = success
+    result_msg = (
+        "Cloned successfully (sparse)"
+        if success
+        else f"Checkout failed: {stderr or stdout}"
+    )
 
-    return True, "Cloned successfully (sparse)"
+    return result_success, result_msg
 
 
 def _clone_full_repo(
@@ -177,6 +175,64 @@ def _clone_full_repo(
         return False, f"Clone failed: {error_msg}"
 
 
+def _process_library_update(
+    lib: dict, libraries_path: Path, progress, verbose: bool
+) -> tuple[str, str, bool]:
+    """Process a single library update and return result."""
+    name = lib.get("name")
+    lib_type = lib.get("type", "git")
+    enabled = lib.get("enabled", True)
+
+    if not enabled:
+        if verbose:
+            display.text(f"Skipping disabled library: {name}", style="dim")
+        return (name, "Skipped (disabled)", False)
+
+    if lib_type == "static":
+        if verbose:
+            display.text(
+                f"Skipping static library: {name} (no sync needed)", style="dim"
+            )
+        return (name, "N/A (static)", True)
+
+    # Handle git libraries
+    url = lib.get("url")
+    branch = lib.get("branch")
+    directory = lib.get("directory", "library")
+
+    task = progress.add_task(f"Updating {name}...", total=None)
+    target_path = libraries_path / name
+    success, message = _clone_or_pull_repo(name, url, target_path, branch, directory)
+    progress.remove_task(task)
+
+    if verbose:
+        if success:
+            display.display_success(f"{name}: {message}")
+        else:
+            display.display_error(f"{name}: {message}")
+
+    return (name, message, success)
+
+
+def _display_update_summary(results: list[tuple[str, str, bool]]) -> None:
+    """Display update summary."""
+    total = len(results)
+    successful = sum(1 for _, _, success in results if success)
+
+    display.text("")
+    if successful == total:
+        display.text(
+            f"All libraries updated successfully ({successful}/{total})", style="green"
+        )
+    elif successful > 0:
+        display.text(
+            f"Partially successful: {successful}/{total} libraries updated",
+            style="yellow",
+        )
+    else:
+        display.text("Failed to update libraries", style="red")
+
+
 @app.command()
 def update(
     library_name: str | None = Argument(
@@ -207,56 +263,14 @@ def update(
             return
 
     libraries_path = config.get_libraries_path()
-
-    # Create results table
     results = []
 
     with display.progress(
         SpinnerColumn(), TextColumn("[progress.description]{task.description}")
     ) as progress:
         for lib in libraries:
-            name = lib.get("name")
-            lib_type = lib.get("type", "git")
-            enabled = lib.get("enabled", True)
-
-            if not enabled:
-                if verbose:
-                    display.text(f"Skipping disabled library: {name}", style="dim")
-                results.append((name, "Skipped (disabled)", False))
-                continue
-
-            # Skip static libraries (no sync needed)
-            if lib_type == "static":
-                if verbose:
-                    display.text(
-                        f"Skipping static library: {name} (no sync needed)", style="dim"
-                    )
-                results.append((name, "N/A (static)", True))
-                continue
-
-            # Handle git libraries
-            url = lib.get("url")
-            branch = lib.get("branch")
-            directory = lib.get("directory", "library")
-
-            task = progress.add_task(f"Updating {name}...", total=None)
-
-            # Target path: ~/.config/boilerplates/libraries/{name}/
-            target_path = libraries_path / name
-
-            # Clone or pull the repository with sparse-checkout if directory is specified
-            success, message = _clone_or_pull_repo(
-                name, url, target_path, branch, directory
-            )
-
-            results.append((name, message, success))
-            progress.remove_task(task)
-
-            if verbose:
-                if success:
-                    display.display_success(f"{name}: {message}")
-                else:
-                    display.display_error(f"{name}: {message}")
+            result = _process_library_update(lib, libraries_path, progress, verbose)
+            results.append(result)
 
     # Display summary table
     if not verbose:
@@ -264,22 +278,69 @@ def update(
             "Library Update Summary", results, columns=("Library", "Status")
         )
 
-    # Summary
-    total = len(results)
-    successful = sum(1 for _, _, success in results if success)
+    _display_update_summary(results)
 
-    display.text("")
-    if successful == total:
-        display.text(
-            f"All libraries updated successfully ({successful}/{total})", style="green"
-        )
-    elif successful > 0:
-        display.text(
-            f"Partially successful: {successful}/{total} libraries updated",
-            style="yellow",
-        )
+
+def _get_library_path_for_git(lib: dict, libraries_path: Path, name: str) -> Path:
+    """Get library path for git library type."""
+    directory = lib.get("directory", "library")
+    library_base = libraries_path / name
+    if directory and directory != ".":
+        return library_base / directory
     else:
-        display.text("Failed to update libraries", style="red")
+        return library_base
+
+
+def _get_library_path_for_static(lib: dict, config: ConfigManager) -> Path:
+    """Get library path for static library type."""
+    url_or_path = lib.get("path", "")
+    library_path = Path(url_or_path).expanduser()
+    if not library_path.is_absolute():
+        library_path = (config.config_path.parent / library_path).resolve()
+    return library_path
+
+
+def _get_library_info(
+    lib: dict, config: ConfigManager, libraries_path: Path
+) -> tuple[str, str, str, str, bool]:
+    """Extract library information based on type."""
+    name = lib.get("name", "")
+    lib_type = lib.get("type", "git")
+    enabled = lib.get("enabled", True)
+
+    if lib_type == "git":
+        url_or_path = lib.get("url", "")
+        branch = lib.get("branch", "main")
+        directory = lib.get("directory", "library")
+        library_path = _get_library_path_for_git(lib, libraries_path, name)
+        exists = library_path.exists()
+
+    elif lib_type == "static":
+        url_or_path = lib.get("path", "")
+        branch = "-"
+        directory = "-"
+        library_path = _get_library_path_for_static(lib, config)
+        exists = library_path.exists()
+
+    else:
+        # Unknown type
+        url_or_path = "<unknown type>"
+        branch = "-"
+        directory = "-"
+        exists = False
+
+    # Build status string
+    status_parts = []
+    if not enabled:
+        status_parts.append("[dim]disabled[/dim]")
+    elif exists:
+        status_parts.append("[green]available[/green]")
+    else:
+        status_parts.append("[yellow]not found[/yellow]")
+
+    status = " ".join(status_parts)
+
+    return url_or_path, branch, directory, lib_type, status
 
 
 @app.command()
@@ -309,52 +370,9 @@ def list() -> None:
 
     for lib in libraries:
         name = lib.get("name", "")
-        lib_type = lib.get("type", "git")
-        enabled = lib.get("enabled", True)
-
-        if lib_type == "git":
-            url_or_path = lib.get("url", "")
-            branch = lib.get("branch", "main")
-            directory = lib.get("directory", "library")
-
-            # Check if library exists locally
-            library_base = libraries_path / name
-            if directory and directory != ".":
-                library_path = library_base / directory
-            else:
-                library_path = library_base
-            exists = library_path.exists()
-
-        elif lib_type == "static":
-            url_or_path = lib.get("path", "")
-            branch = "-"
-            directory = "-"
-
-            # Check if static path exists
-            library_path = Path(url_or_path).expanduser()
-            if not library_path.is_absolute():
-                library_path = (config.config_path.parent / library_path).resolve()
-            exists = library_path.exists()
-
-        else:
-            # Unknown type
-            url_or_path = "<unknown type>"
-            branch = "-"
-            directory = "-"
-            exists = False
-
-        type_display = lib_type
-
-        status_parts = []
-        if not enabled:
-            status_parts.append("[dim]disabled[/dim]")
-        elif exists:
-            status_parts.append("[green]available[/green]")
-        else:
-            status_parts.append("[yellow]not found[/yellow]")
-
-        status = " ".join(status_parts)
-
+        url_or_path, branch, directory, type_display, status = _get_library_info(
+            lib, config, libraries_path
+        )
         table.add_row(name, url_or_path, branch, directory, type_display, status)
 
     display._print_table(table)
@@ -440,7 +458,6 @@ def remove(
             library_path = libraries_path / name
 
             if library_path.exists():
-
                 shutil.rmtree(library_path)
                 display.display_success(f"Deleted local files at {library_path}")
             else:

+ 99 - 86
cli/core/template/template.py

@@ -706,28 +706,15 @@ class Template:
             keep_trailing_newline=False,
         )
 
-    def render(
-        self, variables: VariableCollection, debug: bool = False
-    ) -> tuple[dict[str, str], dict[str, Any]]:
-        """Render all .j2 files in the template directory.
-
-        Args:
-            variables: VariableCollection with values to use for rendering
-            debug: Enable debug mode with verbose output
-
-        Returns:
-            Tuple of (rendered_files, variable_values) where variable_values includes autogenerated values
-        """
-        # Use get_satisfied_values() to exclude variables from sections with unsatisfied dependencies
-        variable_values = variables.get_satisfied_values()
-
-        # Auto-generate values for autogenerated variables that are empty
+    def _generate_autogenerated_values(
+        self, variables: VariableCollection, variable_values: dict
+    ) -> None:
+        """Generate values for autogenerated variables that are empty."""
         for section in variables.get_sections().values():
             for var_name, variable in section.variables.items():
                 if variable.autogenerated and (
                     variable.value is None or variable.value == ""
                 ):
-                    # Generate a secure random string (32 characters by default)
                     alphabet = string.ascii_letters + string.digits
                     generated_value = "".join(
                         secrets.choice(alphabet) for _ in range(32)
@@ -735,6 +722,8 @@ class Template:
                     variable_values[var_name] = generated_value
                     logger.debug(f"Auto-generated value for variable '{var_name}'")
 
+    def _log_render_start(self, debug: bool, variable_values: dict) -> None:
+        """Log rendering start information."""
         if debug:
             logger.info(f"Rendering template '{self.id}' in debug mode")
             logger.info(f"Available variables: {sorted(variable_values.keys())}")
@@ -744,66 +733,111 @@ class Template:
                 f"Rendering template '{self.id}' with variables: {variable_values}"
             )
 
+    def _render_jinja2_file(
+        self, template_file, variable_values: dict, available_vars: set, debug: bool
+    ) -> str:
+        """Render a single Jinja2 template file."""
+        if debug:
+            logger.info(f"Rendering Jinja2 template: {template_file.relative_path}")
+
+        template = self.jinja_env.get_template(str(template_file.relative_path))
+        rendered_content = template.render(**variable_values)
+        rendered_content = self._sanitize_content(
+            rendered_content, template_file.output_path
+        )
+
+        if debug:
+            logger.info(
+                f"Successfully rendered: {template_file.relative_path} -> {template_file.output_path}"
+            )
+
+        return rendered_content
+
+    def _handle_jinja2_error(
+        self,
+        e: Exception,
+        template_file,
+        available_vars: set,
+        variable_values: dict,
+        debug: bool,
+    ) -> None:
+        """Handle Jinja2 rendering errors."""
+        error_msg, line_num, col, context_lines, suggestions = (
+            TemplateErrorHandler.parse_jinja_error(
+                e, template_file, self.template_dir, available_vars
+            )
+        )
+        logger.error(
+            f"Error rendering template file {template_file.relative_path}: {error_msg}"
+        )
+
+        raise TemplateRenderError(
+            message=error_msg,
+            file_path=str(template_file.relative_path),
+            line_number=line_num,
+            column=col,
+            context_lines=context_lines,
+            variable_context={k: str(v) for k, v in variable_values.items()}
+            if debug
+            else {},
+            suggestions=suggestions,
+            original_error=e,
+        ) from e
+
+    def _render_static_file(self, template_file, debug: bool) -> str:
+        """Read and return content of a static file."""
+        file_path = self.template_dir / template_file.relative_path
+        if debug:
+            logger.info(f"Copying static file: {template_file.relative_path}")
+
+        try:
+            with open(file_path, encoding="utf-8") as f:
+                return f.read()
+        except OSError as e:
+            logger.error(f"Error reading static file {file_path}: {e}")
+            raise TemplateRenderError(
+                message=f"Error reading static file: {e}",
+                file_path=str(template_file.relative_path),
+                suggestions=["Check that the file exists and has read permissions"],
+                original_error=e,
+            ) from e
+
+    def render(
+        self, variables: VariableCollection, debug: bool = False
+    ) -> tuple[dict[str, str], dict[str, Any]]:
+        """Render all .j2 files in the template directory.
+
+        Args:
+            variables: VariableCollection with values to use for rendering
+            debug: Enable debug mode with verbose output
+
+        Returns:
+            Tuple of (rendered_files, variable_values) where variable_values includes autogenerated values
+        """
+        variable_values = variables.get_satisfied_values()
+        self._generate_autogenerated_values(variables, variable_values)
+        self._log_render_start(debug, variable_values)
+
         rendered_files = {}
         available_vars = set(variable_values.keys())
 
-        for template_file in self.template_files:  # Iterate over TemplateFile objects
+        for template_file in self.template_files:
             if template_file.file_type == "j2":
                 try:
-                    if debug:
-                        logger.info(
-                            f"Rendering Jinja2 template: {template_file.relative_path}"
-                        )
-
-                    template = self.jinja_env.get_template(
-                        str(template_file.relative_path)
-                    )  # Use lazy-loaded jinja_env
-                    rendered_content = template.render(**variable_values)
-
-                    # Sanitize the rendered content to remove excessive blank lines
-                    rendered_content = self._sanitize_content(
-                        rendered_content, template_file.output_path
+                    content = self._render_jinja2_file(
+                        template_file, variable_values, available_vars, debug
                     )
-                    rendered_files[str(template_file.output_path)] = rendered_content
-
-                    if debug:
-                        logger.info(
-                            f"Successfully rendered: {template_file.relative_path} -> {template_file.output_path}"
-                        )
-
+                    rendered_files[str(template_file.output_path)] = content
                 except (
                     UndefinedError,
                     Jinja2TemplateSyntaxError,
                     Jinja2TemplateNotFound,
                     Jinja2TemplateError,
                 ) as e:
-                    # Parse Jinja2 error to extract detailed information
-                    error_msg, line_num, col, context_lines, suggestions = (
-                        TemplateErrorHandler.parse_jinja_error(
-                            e, template_file, self.template_dir, available_vars
-                        )
-                    )
-
-                    logger.error(
-                        f"Error rendering template file {template_file.relative_path}: {error_msg}"
+                    self._handle_jinja2_error(
+                        e, template_file, available_vars, variable_values, debug
                     )
-
-                    # Create enhanced TemplateRenderError with all context
-                    raise TemplateRenderError(
-                        message=error_msg,
-                        file_path=str(template_file.relative_path),
-                        line_number=line_num,
-                        column=col,
-                        context_lines=context_lines,
-                        variable_context={k: str(v) for k, v in variable_values.items()}
-                        if debug
-                        else {},
-                        suggestions=suggestions,
-                        original_error=e,
-                    ) from e
-
                 except Exception as e:
-                    # Catch any other unexpected errors
                     logger.error(
                         f"Unexpected error rendering template file {template_file.relative_path}: {e}"
                     )
@@ -815,30 +849,9 @@ class Template:
                         ],
                         original_error=e,
                     ) from e
-
             elif template_file.file_type == "static":
-                # For static files, just read their content and add to rendered_files
-                # This ensures static files are also part of the output dictionary
-                file_path = self.template_dir / template_file.relative_path
-                try:
-                    if debug:
-                        logger.info(
-                            f"Copying static file: {template_file.relative_path}"
-                        )
-
-                    with open(file_path, encoding="utf-8") as f:
-                        content = f.read()
-                        rendered_files[str(template_file.output_path)] = content
-                except OSError as e:
-                    logger.error(f"Error reading static file {file_path}: {e}")
-                    raise TemplateRenderError(
-                        message=f"Error reading static file: {e}",
-                        file_path=str(template_file.relative_path),
-                        suggestions=[
-                            "Check that the file exists and has read permissions"
-                        ],
-                        original_error=e,
-                    ) from e
+                content = self._render_static_file(template_file, debug)
+                rendered_files[str(template_file.output_path)] = content
 
         return rendered_files, variable_values
 

+ 17 - 15
cli/core/template/variable.py

@@ -319,29 +319,31 @@ class Variable:
 
         # Type-specific handlers
         if self.type == "enum":
-            if not self.options:
-                return typed
-            return (
-                self.options[0]
-                if typed is None or str(typed) not in self.options
-                else str(typed)
+            result = (
+                typed
+                if not self.options
+                else (
+                    self.options[0]
+                    if typed is None or str(typed) not in self.options
+                    else str(typed)
+                )
             )
-
-        if self.type == "bool":
-            return (
+        elif self.type == "bool":
+            result = (
                 typed
                 if isinstance(typed, bool)
                 else (None if typed is None else bool(typed))
             )
-
-        if self.type == "int":
+        elif self.type == "int":
             try:
-                return int(typed) if typed not in (None, "") else None
+                result = int(typed) if typed not in (None, "") else None
             except Exception:
-                return None
+                result = None
+        else:
+            # Default: return string or None
+            result = None if typed is None else str(typed)
 
-        # Default: return string or None
-        return None if typed is None else str(typed)
+        return result
 
     def get_prompt_text(self) -> str:
         """Get formatted prompt text for interactive input.

+ 152 - 116
cli/core/template/variable_collection.py

@@ -206,6 +206,48 @@ class VariableCollection:
             # Old format: section name (backwards compatibility)
             return (need_str.strip(), None)
 
+    def _check_section_need(self, section_name: str) -> bool:
+        """Check if a section dependency is satisfied."""
+        section = self._sections.get(section_name)
+        if not section:
+            logger.warning(f"Need references missing section '{section_name}'")
+            return False
+        return section.is_enabled()
+
+    def _check_value_match(
+        self, actual_value: Any, expected: Any, var_type: str
+    ) -> bool:
+        """Check if actual value matches expected value based on variable type."""
+        if var_type == "bool":
+            return bool(actual_value) == bool(expected)
+        return actual_value is not None and str(actual_value) == str(expected)
+
+    def _check_variable_need(
+        self, var_name: str, expected_value: Any, variable
+    ) -> bool:
+        """Check if a variable dependency is satisfied."""
+        try:
+            actual_value = variable.convert(variable.value)
+
+            # Handle multiple expected values
+            if isinstance(expected_value, list):
+                for expected in expected_value:
+                    expected_converted = variable.convert(expected)
+                    if self._check_value_match(
+                        actual_value, expected_converted, variable.type
+                    ):
+                        return True
+                return False
+
+            # Single expected value
+            expected_converted = variable.convert(expected_value)
+            return self._check_value_match(
+                actual_value, expected_converted, variable.type
+            )
+        except Exception as e:
+            logger.debug(f"Failed to compare need for '{var_name}': {e}")
+            return False
+
     def _is_need_satisfied(self, need_str: str) -> bool:
         """Check if a single need condition is satisfied.
 
@@ -217,57 +259,17 @@ class VariableCollection:
         """
         var_or_section, expected_value = self._parse_need(need_str)
 
+        # Old format: section name check
         if expected_value is None:
-            # Old format: check if section is enabled (backwards compatibility)
-            section = self._sections.get(var_or_section)
-            if not section:
-                logger.warning(f"Need references missing section '{var_or_section}'")
-                return False
-            return section.is_enabled()
-        else:
-            # New format: check if variable has expected value(s)
-            variable = self._variable_map.get(var_or_section)
-            if not variable:
-                logger.warning(f"Need references missing variable '{var_or_section}'")
-                return False
+            return self._check_section_need(var_or_section)
 
-            # Convert actual value for comparison
-            try:
-                actual_value = variable.convert(variable.value)
-
-                # Handle multiple expected values (comma-separated in needs)
-                if isinstance(expected_value, list):
-                    # Check if actual value matches any of the expected values
-                    for expected in expected_value:
-                        expected_converted = variable.convert(expected)
-
-                        # Handle boolean comparisons specially
-                        if variable.type == "bool":
-                            if bool(actual_value) == bool(expected_converted):
-                                return True
-                        # String comparison for other types
-                        elif actual_value is not None and str(actual_value) == str(
-                            expected_converted
-                        ):
-                            return True
-                    return False  # None of the expected values matched
-                else:
-                    # Single expected value (original behavior)
-                    expected_converted = variable.convert(expected_value)
-
-                    # Handle boolean comparisons specially
-                    if variable.type == "bool":
-                        return bool(actual_value) == bool(expected_converted)
-
-                    # String comparison for other types
-                    return (
-                        str(actual_value) == str(expected_converted)
-                        if actual_value is not None
-                        else False
-                    )
-            except Exception as e:
-                logger.debug(f"Failed to compare need '{need_str}': {e}")
-                return False
+        # New format: variable value check
+        variable = self._variable_map.get(var_or_section)
+        if not variable:
+            logger.warning(f"Need references missing variable '{var_or_section}'")
+            return False
+
+        return self._check_variable_need(var_or_section, expected_value, variable)
 
     def _validate_dependencies(self) -> None:
         """Validate section dependencies for cycles and missing references.
@@ -723,23 +725,32 @@ class VariableCollection:
 
         return successful
 
-    def validate_all(self) -> None:
-        """Validate all variables in the collection.
-
-        Validates:
-        - All variables in enabled sections with satisfied dependencies
-        - Required variables even if their section is disabled (but dependencies must be satisfied)
-        - CLI-provided bool variables with unsatisfied dependencies
-        """
+    def _collect_unmet_needs(self, section, var_name: str, variable) -> set[str]:
+        """Collect unmet needs for a variable."""
+        section_satisfied = self.is_section_satisfied(section.key)
+        var_satisfied = self.is_variable_satisfied(var_name)
+        unmet_needs = set()
+
+        if not section_satisfied:
+            for need in section.needs:
+                if not self._is_need_satisfied(need):
+                    unmet_needs.add(need)
+        if not var_satisfied:
+            for need in variable.needs:
+                if not self._is_need_satisfied(need):
+                    unmet_needs.add(need)
+
+        return unmet_needs
+
+    def _validate_cli_bool_variables(self) -> list[str]:
+        """Validate CLI-provided bool variables with unsatisfied dependencies."""
         errors: list[str] = []
 
-        # First, check for CLI-provided bool variables with unsatisfied dependencies
         for section_key, section in self._sections.items():
             section_satisfied = self.is_section_satisfied(section_key)
             is_enabled = section.is_enabled()
 
             for var_name, variable in section.variables.items():
-                # Check CLI-provided bool variables with unsatisfied dependencies
                 if (
                     variable.type == "bool"
                     and variable.origin == "cli"
@@ -748,17 +759,9 @@ class VariableCollection:
                     var_satisfied = self.is_variable_satisfied(var_name)
 
                     if not section_satisfied or not is_enabled or not var_satisfied:
-                        # Build error message with unmet needs (use set to avoid duplicates)
-                        unmet_needs = set()
-                        if not section_satisfied:
-                            for need in section.needs:
-                                if not self._is_need_satisfied(need):
-                                    unmet_needs.add(need)
-                        if not var_satisfied:
-                            for need in variable.needs:
-                                if not self._is_need_satisfied(need):
-                                    unmet_needs.add(need)
-
+                        unmet_needs = self._collect_unmet_needs(
+                            section, var_name, variable
+                        )
                         needs_str = (
                             ", ".join(sorted(unmet_needs))
                             if unmet_needs
@@ -768,58 +771,91 @@ class VariableCollection:
                             f"{section.key}.{var_name} (set via CLI to {variable.value} but requires: {needs_str})"
                         )
 
-        # Then validate all other variables
-        for section_key, section in self._sections.items():
-            # Skip sections with unsatisfied dependencies (even for required variables)
-            if not self.is_section_satisfied(section_key):
-                logger.debug(
-                    f"Skipping validation for section '{section_key}' - dependencies not satisfied"
-                )
-                continue
+        return errors
+
+    def _validate_variable(self, section, var_name: str, variable) -> str | None:
+        """Validate a single variable and return error message if invalid."""
+        try:
+            # Skip autogenerated variables when empty
+            if variable.autogenerated and not variable.value:
+                return None
+
+            # Check required fields
+            if variable.value is None:
+                return self._validate_none_value(section, var_name, variable)
+
+            # Validate typed value
+            typed = variable.convert(variable.value)
+            if variable.type not in ("bool",) and not typed:
+                return self._validate_empty_value(section, var_name, variable)
+
+        except ValueError as e:
+            return f"{section.key}.{var_name} (invalid format: {e})"
+
+        return None
+
+    def _validate_none_value(self, section, var_name: str, variable) -> str | None:
+        """Validate when variable value is None."""
+        # Optional variables can be None/empty
+        if hasattr(variable, "optional") and variable.optional:
+            return None
+        if variable.is_required():
+            return f"{section.key}.{var_name} (required - no default provided)"
+        return None
+
+    def _validate_empty_value(self, section, var_name: str, variable) -> str | None:
+        """Validate when variable value is empty."""
+        msg = f"{section.key}.{var_name}"
+        if variable.is_required():
+            return f"{msg} (required - cannot be empty)"
+        return f"{msg} (empty)"
+
+    def _validate_section_variables(self, section_key: str, section) -> list[str]:
+        """Validate all variables in a section."""
+        errors: list[str] = []
 
-            # Check if section is enabled
-            is_enabled = section.is_enabled()
+        # Skip sections with unsatisfied dependencies
+        if not self.is_section_satisfied(section_key):
+            logger.debug(
+                f"Skipping validation for section '{section_key}' - dependencies not satisfied"
+            )
+            return errors
+
+        is_enabled = section.is_enabled()
 
+        if not is_enabled:
+            logger.debug(
+                f"Section '{section_key}' is disabled - validating only required variables"
+            )
+
+        # Validate variables in the section
+        for var_name, variable in section.variables.items():
+            # Skip all variables in disabled sections
             if not is_enabled:
-                logger.debug(
-                    f"Section '{section_key}' is disabled - validating only required variables"
-                )
+                continue
 
-            # Validate variables in the section
-            for var_name, variable in section.variables.items():
-                # Skip all variables (including required ones) in disabled sections
-                # Required variables are only required when their section is actually enabled
-                if not is_enabled:
-                    continue
+            error = self._validate_variable(section, var_name, variable)
+            if error:
+                errors.append(error)
 
-                try:
-                    # Skip autogenerated variables when empty
-                    if variable.autogenerated and not variable.value:
-                        continue
-
-                    # Check required fields
-                    if variable.value is None:
-                        # Optional variables can be None/empty
-                        if hasattr(variable, "optional") and variable.optional:
-                            continue
-                        if variable.is_required():
-                            errors.append(
-                                f"{section.key}.{var_name} (required - no default provided)"
-                            )
-                        continue
-
-                    # Validate typed value
-                    typed = variable.convert(variable.value)
-                    if variable.type not in ("bool",) and not typed:
-                        msg = f"{section.key}.{var_name}"
-                        errors.append(
-                            f"{msg} (required - cannot be empty)"
-                            if variable.is_required()
-                            else f"{msg} (empty)"
-                        )
+        return errors
+
+    def validate_all(self) -> None:
+        """Validate all variables in the collection.
 
-                except ValueError as e:
-                    errors.append(f"{section.key}.{var_name} (invalid format: {e})")
+        Validates:
+        - All variables in enabled sections with satisfied dependencies
+        - Required variables even if their section is disabled (but dependencies must be satisfied)
+        - CLI-provided bool variables with unsatisfied dependencies
+        """
+        errors: list[str] = []
+
+        # First, check for CLI-provided bool variables with unsatisfied dependencies
+        errors.extend(self._validate_cli_bool_variables())
+
+        # Then validate all other variables
+        for section_key, section in self._sections.items():
+            errors.extend(self._validate_section_variables(section_key, section))
 
         if errors:
             error_msg = "Variable validation failed: " + ", ".join(errors)

+ 46 - 38
cli/core/template/variable_section.py

@@ -132,43 +132,31 @@ class VariableSection:
 
         return cloned
 
-    def sort_variables(self, is_need_satisfied_func=None) -> None:
-        """Sort variables within section for optimal display and user interaction.
-
-        Current sorting strategy:
-        - Variables with no dependencies come first
-        - Variables that depend on others come after their dependencies (topological sort)
-        - Original order is preserved for variables at the same dependency level
-
-        Future sorting strategies can be added here (e.g., by type, required first, etc.)
-
-        Args:
-            is_need_satisfied_func: Optional function to check if a variable need is satisfied
-                                   (reserved for future use in conditional sorting)
-        """
-        if not self.variables:
-            return
-
-        # Build dependency graph
-        var_list = list(self.variables.keys())
+    def _build_dependency_graph(self, var_list: list[str]) -> dict[str, list[str]]:
+        """Build dependency graph for variables in this section."""
         var_set = set(var_list)
-
-        # For each variable, find which OTHER variables in THIS section it depends on
         dependencies = {var_name: [] for var_name in var_list}
+
         for var_name in var_list:
             variable = self.variables[var_name]
-            if variable.needs:
-                for need in variable.needs:
-                    # Parse need format: "variable_name=value"
-                    dep_var = need.split("=")[0] if "=" in need else need
-                    # Only track dependencies within THIS section
-                    if dep_var in var_set and dep_var != var_name:
-                        dependencies[var_name].append(dep_var)
-
-        # Topological sort using Kahn's algorithm
+            if not variable.needs:
+                continue
+
+            for need in variable.needs:
+                # Parse need format: "variable_name=value"
+                dep_var = need.split("=")[0] if "=" in need else need
+                # Only track dependencies within THIS section
+                if dep_var in var_set and dep_var != var_name:
+                    dependencies[var_name].append(dep_var)
+
+        return dependencies
+
+    def _topological_sort(
+        self, var_list: list[str], dependencies: dict[str, list[str]]
+    ) -> list[str]:
+        """Perform topological sort using Kahn's algorithm."""
         in_degree = {var_name: len(deps) for var_name, deps in dependencies.items()}
         queue = [var for var, degree in in_degree.items() if degree == 0]
-        # Preserve original order for variables with same dependency level
         queue.sort(key=lambda v: var_list.index(v))
         result = []
 
@@ -186,12 +174,32 @@ class VariableSection:
 
         # If not all variables were sorted (cycle), append remaining in original order
         if len(result) != len(var_list):
-            for var_name in var_list:
-                if var_name not in result:
-                    result.append(var_name)
+            result.extend(var_name for var_name in var_list if var_name not in result)
+
+        return result
+
+    def sort_variables(self, is_need_satisfied_func=None) -> None:
+        """Sort variables within section for optimal display and user interaction.
+
+        Current sorting strategy:
+        - Variables with no dependencies come first
+        - Variables that depend on others come after their dependencies (topological sort)
+        - Original order is preserved for variables at the same dependency level
+
+        Future sorting strategies can be added here (e.g., by type, required first, etc.)
+
+        Args:
+            is_need_satisfied_func: Optional function to check if a variable need is satisfied
+                                   (reserved for future use in conditional sorting)
+        """
+        if not self.variables:
+            return
+
+        var_list = list(self.variables.keys())
+        dependencies = self._build_dependency_graph(var_list)
+        result = self._topological_sort(var_list, dependencies)
 
         # Rebuild variables OrderedDict in new order
-        sorted_vars = OrderedDict()
-        for var_name in result:
-            sorted_vars[var_name] = self.variables[var_name]
-        self.variables = sorted_vars
+        self.variables = OrderedDict(
+            (var_name, self.variables[var_name]) for var_name in result
+        )