pkg/list/list_components.go (1)

51-78: Consider adding function documentation

Adding a comment or docstring to resolveKey would improve readability and help others understand its purpose.

cmd/list_components.go (1)

42-44: Validate list configuration before use

Ensure cliConfig.Components.List is properly initialized to prevent unexpected errors when listing components.

pkg/schema/schema.go (1)

102-104: LGTM! Consider adding field documentation.

The ListColumnConfig type is well-structured and follows consistent patterns. Consider adding documentation comments to describe the expected format for the Value field, especially if it supports templating or special syntax.

Example documentation:

 type ListColumnConfig struct {
+    // Name is the display name of the column
     Name  string `yaml:"name" json:"name" mapstructure:"name"`
+    // Value is the template expression to extract the column value
     Value string `yaml:"value" json:"value" mapstructure:"value"`
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL

pkg/list/list_components.go (2)

48-48: Consider using '%v' for fmt.Sprintf to handle all value types

Using %v in fmt.Sprintf ensures that all types of value are formatted correctly, even if they aren't strings.

Apply this diff to update the code:

-rowData = append(rowData, fmt.Sprintf("%s", value))
+rowData = append(rowData, fmt.Sprintf("%v", value))

---

102-103: Improve error message for more context

Including the invalid value in the error message can help users identify formatting issues more easily.

Apply this diff to enhance the error message:

-return "", fmt.Errorf("invalid value format for column name %s", v.Name)
+return "", fmt.Errorf("invalid value format '%s' for column name '%s'", v.Value, v.Name)

Configuration used: .coderabbit.yaml
Review profile: CHILL

pkg/list/list_components.go (3)

Line range hint 16-52: STRONG WORK on the error handling, warrior! Let's make it even more robust.

The function handles type assertions well, but we should validate the input parameters for additional strength.

Consider these battle-hardening improvements:

 func getStackComponents(stackData any, listFields []string) ([]string, error) {
+    if len(listFields) == 0 {
+        return nil, fmt.Errorf("listFields cannot be empty")
+    }
+
     stackMap, ok := stackData.(map[string]any)
     if !ok {
         return nil, fmt.Errorf("could not parse stacks")
     }

Also, pre-allocate the result slice with a capacity hint to optimize memory allocation:

-    result := make([]string, 0)
+    result := make([]string, 0, len(uniqueKeys))

---

54-81: Well-crafted battle plan for key resolution, soldier! Let's document it properly.

The function handles nested key resolution effectively, but could use more detailed documentation for future warriors.

Consider enhancing the documentation:

-// resolveKey resolves a key from a map, supporting nested keys with dot notation
+// resolveKey resolves a key from a map using dot notation for nested access.
+// Example:
+//   data := map[string]any{"vars": map[string]any{"tenant": "sparta"}}
+//   value, found := resolveKey(data, "vars.tenant") // Returns "sparta", true
+// Returns the resolved value and true if found, or nil and false if not found.
 func resolveKey(data map[string]any, key string) (any, bool) {

---

180-185: A simple but vital function for our formation! Let's make it faster.

The current implementation works but could be more efficient for longer padding requirements.

Consider using strings.Repeat for better performance:

 func padToWidth(str string, width int) string {
-    for len(str) < width {
-        str += " "
-    }
-    return str
+    if len(str) >= width {
+        return str
+    }
+    return str + strings.Repeat(" ", width-len(str))
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL

pkg/list/list_components.go (3)

66-93: Strengthen our defenses with better error context!

While the function is well-structured, consider enhancing error reporting by including the full key path in the error context when traversal fails.

-            return nil, false
+            return nil, fmt.Errorf("failed to resolve key '%s' at part '%s'", key, part)

---

146-167: Optimize our battle formation!

The function performs well, but we can improve memory allocation efficiency by pre-allocating slices with capacity hints.

-    colWidths := make([]int, len(header))
+    colWidths := make([]int, len(header), len(header))

---

200-227: Document our battle strategy!

While the implementation is solid, consider adding godoc-style documentation to explain the return values and their significance.

+// FilterAndListComponents processes and displays components in a formatted table.
+// Returns an empty string on success or "No components found" when appropriate.
+// Error is returned if any processing step fails.
 func FilterAndListComponents(stackFlag string, stacksMap map[string]any, listConfig schema.ListConfig) (string, error) {

Configuration used: .coderabbit.yaml
Review profile: CHILL

pkg/list/list_components.go (6)

15-24: By the gods, these constants and types are well-structured!

The color constants and tableData struct provide a solid foundation for table formatting. However, a true warrior documents their arsenal.

Consider adding documentation comments for the tableData struct fields:

 type tableData struct {
+    // header contains the column names
     header    []string
+    // rows contains the table data
     rows      [][]string
+    // colWidths contains the width of each column
     colWidths []int
 }

---

65-93: A valiant attempt at key resolution, but let's sharpen our sword!

The function handles nested key traversal well, but could provide more context in error cases.

Consider enhancing error context:

 func resolveKey(data map[string]any, key string) (any, bool) {
     key = strings.TrimPrefix(key, ".")
     parts := strings.Split(key, ".")
     current := data
+    path := ""
     for i, part := range parts {
+        path += "." + part
         if i == len(parts)-1 {
             if value, exists := current[part]; exists {
                 return value, true
             }
-            return nil, false
+            return nil, fmt.Errorf("key not found at path: %s", strings.TrimPrefix(path, "."))
         }
         if nestedMap, ok := current[part].(map[string]any); ok {
             current = nestedMap
         } else {
-            return nil, false
+            return nil, fmt.Errorf("invalid path at: %s", strings.TrimPrefix(path, "."))
         }
     }
     return nil, false
 }

---

95-113: A well-forged function, but let's optimize for battle!

The column parsing logic is sound, but the regular expression compilation could be moved to package level for better performance.

Consider pre-compiling the regex:

+var columnValueRegex = regexp.MustCompile(`\{\{\s*(.*?)\s*\}\}`)
+
 func parseColumns(listConfig schema.ListConfig) ([]string, []string, error) {
     header := make([]string, 0)
     listFields := make([]string, 0)
-    re := regexp.MustCompile(`\{\{\s*(.*?)\s*\}\}`)
 
     for _, col := range listConfig.Columns {
         if col.Value == "" {
             return nil, nil, fmt.Errorf("empty value for column name %s", col.Name)
         }
         header = append(header, col.Name)
-        match := re.FindStringSubmatch(col.Value)
+        match := columnValueRegex.FindStringSubmatch(col.Value)

---

116-151: A strong warrior that now reports all battle casualties!

The error collection implementation is praiseworthy. For larger battles (datasets), we could employ parallel processing.

Consider adding concurrent processing for multiple stacks:

func collectComponentsConcurrently(stacksMap map[string]any, listFields []string) ([][]string, error) {
    var mu sync.Mutex
    var wg sync.WaitGroup
    components := [][]string{}
    errors := []string{}
    
    for stackName, stackData := range stacksMap {
        wg.Add(1)
        go func(name string, data any) {
            defer wg.Done()
            stackComponents, err := getStackComponents(data, listFields)
            mu.Lock()
            defer mu.Unlock()
            if err != nil {
                errors = append(errors, fmt.Sprintf("stack '%s': %v", name, err))
                return
            }
            for _, c := range stackComponents {
                components = append(components, strings.Fields(c))
            }
        }(stackName, stackData)
    }
    wg.Wait()
    
    if len(errors) > 0 {
        return components, fmt.Errorf("errors processing stacks: %s", strings.Join(errors, "; "))
    }
    return components, nil
}

---

177-205: A visually pleasing battle formation, but let's consider the battlefield size!

The table formatting looks great, but we should consider terminal width constraints.

Consider adding terminal width handling:

+import "golang.org/x/term"
+
 func formatTable(data tableData) {
+    width, _, err := term.GetSize(0)
+    if err != nil {
+        width = 80 // fallback width
+    }
+    
+    // Calculate total width including spacing
+    totalWidth := 0
+    for _, w := range data.colWidths {
+        totalWidth += w + 2 // +2 for spacing
+    }
+    
+    // Adjust column widths if necessary
+    if totalWidth > width {
+        ratio := float64(width) / float64(totalWidth)
+        for i := range data.colWidths {
+            data.colWidths[i] = int(float64(data.colWidths[i]) * ratio)
+        }
+    }

---

207-235: A well-orchestrated battle plan, but let's clarify the victory message!

The function orchestrates the process well, but returning an empty string on success might be confusing.

Consider returning a summary message on success:

 func FilterAndListComponents(stackFlag string, stacksMap map[string]any, listConfig schema.ListConfig) (string, error) {
     // ... existing code ...
     
     formatTable(data)
 
-    return "", nil
+    return fmt.Sprintf("Successfully displayed %d components", len(processedComponents)), nil
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL

cmd/list_components.go (2)

Line range hint 43-48: Consider enhancing error handling with more context.

While the function call is correct, we could improve error handling to provide more context about which components failed to process.

-		output, err := l.FilterAndListComponents(stackFlag, abstractFlag, stacksMap, cliConfig.Components.List)
-		if err != nil {
-			u.PrintMessageInColor(fmt.Sprintf("Error: %v"+"\n", err), color.New(color.FgYellow))
-			return
-		}
+		output, err := l.FilterAndListComponents(stackFlag, abstractFlag, stacksMap, cliConfig.Components.List)
+		if err != nil {
+			if strings.Contains(err.Error(), "errors processing stacks") {
+				u.PrintMessageInColor(fmt.Sprintf("Warning: Some stacks failed to process: %v"+"\n", err), color.New(color.FgYellow))
+			} else {
+				u.PrintMessageInColor(fmt.Sprintf("Error: %v"+"\n", err), color.New(color.FgRed))
+				return
+			}
+		}

---

55-55: Enhance the flag description for better clarity.

The help message could be more descriptive to better explain the flag's purpose.

-	listComponentsCmd.PersistentFlags().Bool("abstract", false, "Filter abstract component if true")
+	listComponentsCmd.PersistentFlags().Bool("abstract", false, "Include abstract components in the output (default: only concrete components)")

pkg/list/list_components.go (4)

18-22: Add documentation for the tableData struct.

Consider adding a descriptive comment to explain the purpose of each field in the struct.

+// tableData holds the necessary information for rendering a formatted table
+// header: contains the column names
+// rows: contains the data rows
+// colWidths: contains the width of each column for proper alignment
 type tableData struct {
 	header    []string
 	rows      [][]string
 	colWidths []int
 }

---

69-97: Strengthen error handling in resolveKey function.

The function handles basic cases well, but consider adding validation for empty keys and nil maps.

 func resolveKey(data map[string]any, key string) (any, bool) {
+	if data == nil {
+		return nil, false
+	}
+	if key == "" {
+		return nil, false
+	}
 	// Remove leading dot from the key
 	key = strings.TrimPrefix(key, ".")

---

141-156: Enhance error reporting with stack context.

Consider structuring the error collection to provide more context about which stacks failed.

-		var errors []string
+		type stackError struct {
+			stack string
+			err   error
+		}
+		var errors []stackError
 		for stackName, stackData := range stacksMap {
 			stackComponents, err := getStackComponents(stackData, abstractFlag, listFields)
 			if err != nil {
-				errors = append(errors, err.Error())
+				errors = append(errors, stackError{stack: stackName, err: err})
 				continue // Skip invalid stacks
 			}
 		}
 		if len(errors) > 0 {
-			return components, fmt.Errorf("errors processing stacks: %s", strings.Join(errors, "; "))
+			var errMsgs []string
+			for _, e := range errors {
+				errMsgs = append(errMsgs, fmt.Sprintf("stack '%s': %v", e.stack, e.err))
+			}
+			return components, fmt.Errorf("errors processing stacks:\n%s", strings.Join(errMsgs, "\n"))
 		}

---

242-270: Consider returning meaningful success information.

The function returns an empty string on success, which could be more informative.

-func FilterAndListComponents(stackFlag string, abstractFlag bool, stacksMap map[string]any, listConfig schema.ListConfig) (string, error) {
+func FilterAndListComponents(stackFlag string, abstractFlag bool, stacksMap map[string]any, listConfig schema.ListConfig) (result struct {
+	ComponentCount int
+	Message       string
+}, error) {
 	// ... existing code ...
-	return "", nil
+	return struct {
+		ComponentCount int
+		Message       string
+	}{
+		ComponentCount: len(processedComponents),
+		Message:       fmt.Sprintf("Successfully listed %d components", len(processedComponents)),
+	}, nil

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

pkg/list/list_components.go (1)

100-122: 🛠️ Refactor suggestion

Optimize our battle strategy with pre-compiled regex!

The regex pattern should be compiled once at the package level for better performance.

Move the regex compilation to package level:

+var columnValueRegex = regexp.MustCompile(`\{\{\s*(.*?)\s*\}\}`)
+
 func parseColumns(listConfig schema.ListConfig) ([]string, []string, error) {
-    re := regexp.MustCompile(`\{\{\s*(.*?)\s*\}\}`)
     // ... rest of the function
-    match := re.FindStringSubmatch(col.Value)
+    match := columnValueRegex.FindStringSubmatch(col.Value)

pkg/list/list_components.go (5)

Line range hint 26-67: By Ares' shield, let's make this function more readable!

The function has good error handling, but we can make it more maintainable with early returns.

Consider this battle-tested structure:

 func getStackComponents(stackData any, abstractFlag bool, listFields []string) ([]string, error) {
     stackMap, ok := stackData.(map[string]any)
     if !ok {
         return nil, fmt.Errorf("could not parse stacks")
     }
 
     componentsMap, ok := stackMap["components"].(map[string]any)
     if !ok {
         return nil, fmt.Errorf("could not parse components")
     }
 
     terraformComponents, ok := componentsMap["terraform"].(map[string]any)
     if !ok {
         return nil, fmt.Errorf("could not parse Terraform components")
     }
 
-    var uniqueKeys []string
-    if abstractFlag {
-        uniqueKeys = lo.Keys(terraformComponents)
-    } else {
-        uniqueKeys = exec.FilterAbstractComponents(terraformComponents)
-    }
+    uniqueKeys := lo.Keys(terraformComponents)
+    if !abstractFlag {
+        uniqueKeys = exec.FilterAbstractComponents(terraformComponents)
+    }

---

70-98: A well-forged function deserves proper documentation!

The resolveKey function is well-implemented, but adding examples would help future warriors understand its usage.

Consider adding this documentation:

+// resolveKey resolves a key from a map using dot notation.
+// Example:
+//   data := map[string]any{"vars": map[string]any{"tenant": "sparta"}}
+//   value, found := resolveKey(data, "vars.tenant") // Returns "sparta", true
 func resolveKey(data map[string]any, key string) (any, bool) {

---

124-159: A mighty implementation worthy of Sparta!

The error handling is robust, especially the collection of errors from multiple stacks.

Consider using a custom error type for better error handling:

type StackProcessingError struct {
    Stack string
    Err   error
}

func (e *StackProcessingError) Error() string {
    return fmt.Sprintf("error processing stack '%s': %v", e.Stack, e.Err)
}

---

161-183: A well-organized battle formation!

The component processing is efficient, but for large datasets, we could parallelize the operations.

Consider using goroutines for parallel processing:

func processComponentsParallel(header []string, components [][]string) ([][]string, []int) {
    // Deduplicate and sort (current implementation)
    uniqueComponents := lo.UniqBy(components, func(item []string) string {
        return strings.Join(item, "\t")
    })
    
    // Calculate column widths in parallel
    colWidths := make([]int, len(header))
    var wg sync.WaitGroup
    for i := range header {
        wg.Add(1)
        go func(colIndex int) {
            defer wg.Done()
            for _, row := range uniqueComponents {
                if len(row[colIndex]) > colWidths[colIndex] {
                    colWidths[colIndex] = len(row[colIndex])
                }
            }
        }(i)
    }
    wg.Wait()
    
    return uniqueComponents, colWidths
}

---

185-288: A visually pleasing battle report!

The table generation is well-implemented with good TTY detection and styling.

Consider making the colors configurable:

+type TableConfig struct {
+    HeaderColor  lipgloss.Color
+    OddRowColor  lipgloss.Color
+    EvenRowColor lipgloss.Color
+    BorderColor  lipgloss.Color
+}
+
+var DefaultTableConfig = TableConfig{
+    HeaderColor:  purple,
+    OddRowColor:  gray,
+    EvenRowColor: lightGray,
+    BorderColor:  gray,
+}

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

-func FilterAndListComponents(stackFlag string, abstractFlag bool, stacksMap map[string]any, listConfig schema.ListConfig) (string, error) {
+func FilterAndListComponents(stackFlag string, abstractFlag bool, stacksMap map[string]any, listConfig schema.ListConfig) (result struct {
+    ComponentCount int
+    Message        string
+}, error) {
     // ... rest of the function
-    return "", nil
+    return struct {
+        ComponentCount int
+        Message        string
+    }{
+        ComponentCount: len(processedComponents),
+        Message:        fmt.Sprintf("Successfully listed %d components", len(processedComponents)),
+    }, nil

cmd/list_components.go (1)

28-28: Consider adding error handling for the flag.

While the flag addition is good, the error from GetBool is being ignored. Consider handling potential flag parsing errors.

-abstractFlag, _ := cmd.Flags().GetBool("abstract")
+abstractFlag, err := cmd.Flags().GetBool("abstract")
+if err != nil {
+    u.PrintMessageInColor(fmt.Sprintf("Error getting abstract flag: %v", err), color.New(color.FgRed))
+    return
+}

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

#!/bin/bash
# Check if TTY-specific handling exists in the codebase
rg -l "isatty|IsTTY" .

# Check for lipgloss usage
rg -l "lipgloss" .

#!/bin/bash
# Check TTY detection implementation in vendor_component_utils.go
rg -A 5 "isatty|IsTTY" ./internal/exec/vendor_component_utils.go

# Check lipgloss formatting in list_components.go
rg -A 10 "lipgloss" ./pkg/list/list_components.go

# Check the FilterAndListComponents function implementation
ast-grep --pattern 'func (l *ListComponents) FilterAndListComponents($$$) ($$$) {
  $$$
}'

#!/bin/bash
# Check for TTY detection in list_components.go
rg -B 5 -A 5 "term\." ./pkg/list/list_components.go

# Check the full implementation context
ast-grep --pattern 'func (l *ListComponents) FilterAndListComponents($_, $_, $_, $_) ($_, $_) {
  $$$
}'

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro