Fix go vet issues

ARCHITECTURE.md

@@ -0,0 +1,95 @@
+# ctop Architecture
+
+## 1. Application Overview and Objectives
+
+`ctop` is a command-line tool that provides a concise and real-time overview of container metrics. It acts as a "top-like" interface for containers, allowing users to monitor CPU, memory, network, and I/O usage at a glance directly from their terminal.
+
+The primary objectives of `ctop` are:
+- **Real-Time Monitoring:** Provide a live, continuously updated view of container performance metrics.
+- **Extensibility:** Support multiple container runtimes (e.g., Docker, runc) through a modular backend system.
+- **Interactivity:** Allow users to sort, filter, and manage containers through an intuitive terminal user interface (TUI).
+- **Lightweight:** Be a minimal, efficient tool that can run easily in various environments.
+
+## 2. Architecture and Design Choices
+
+`ctop` is built with a modular and concurrent architecture to keep the UI responsive while collecting data from multiple sources in the background.
+
+### Core Components
+
+#### a. Connector Interface
+The most critical design choice is the `Connector` interface, which decouples the core application from the container backend. This allows `ctop` to support different container runtimes by providing a specific implementation for each.
+
+- **`Connector` Interface (`connector/main.go`):** Defines the essential methods a backend must provide, such as `All()` to list containers and `Get()` to retrieve a specific container.
+- **`ConnectorSuper` (`connector/main.go`):** A wrapper that provides resilient connection logic, including initial connection and automatic retries on failure.
+- **Implementations:**
+    - `connector/docker.go`: The implementation for the Docker engine.
+    - `connector/runc.go`: The implementation for runc.
+    - `connector/mock.go`: A mock implementation used for development and testing.
+
+#### b. Data Model (`container/` and `models/`)
+The data is structured logically to separate the container's identity from its metrics and metadata.
+
+- **`container.Container` (`container/container.go`):** The central data structure representing a single container. It holds metadata, the latest metrics, and, importantly, references to its specific `Collector` and `Manager`.
+- **`models/`:** This package defines the raw data structures for `Metrics` (CPU, memory, etc.) and `Meta` (name, image, state, etc.), ensuring a clean separation of data from logic.
+
+#### c. Data Collection and Management (`collector/` and `manager/`)
+Each container's lifecycle and data streams are handled by dedicated components.
+
+- **`Collector` (`connector/collector/`):** Responsible for collecting metrics for a single container. Each connector type has a corresponding collector (e.g., `docker.go`, `runc.go`). Collectors typically run in a dedicated goroutine per container, streaming `models.Metrics` back to the main application via channels.
+- **`Manager` (`connector/manager/`):** Provides an interface for performing actions on a container, such as `Start()`, `Stop()`, and `Pause()`.
+
+#### d. Terminal User Interface (TUI)
+The TUI is built using the `termui` library and is composed of several custom widgets.
+
+- **`grid.go`:** Manages the main display, which is a grid of containers. It handles layout, redrawing, and refreshing the container list.
+- **`cwidgets/`:** Contains all the custom, reusable UI components, such as the compact grid view (`cwidgets/compact/`) and the detailed single-container view (`cwidgets/single/`).
+- **`menus.go`:** Defines the logic for interactive menus like Help, Filter, Sort, and Column selection.
+
+### Concurrency Model
+`ctop` is heavily concurrent to ensure a non-blocking UI.
+- The **main goroutine** handles UI rendering and user input events.
+- Each container's **collector runs in its own goroutine**, continuously fetching metrics and sending them back over a channel.
+- The active **connector runs an event-watching goroutine** in the background to listen for container events like `start`, `stop`, and `die`, pushing updates to the UI.
+- **Channels** are the primary means of communication, used for streaming metrics, signaling the need for a UI refresh, and propagating status updates.
+
+## 3. Command-Line Arguments
+
+`ctop` can be configured at startup using the following command-line flags.
+
+| Flag | Type | Default | Description |
+|---|---|---|---|
+| `-v` | bool | `false` | Output version information and exit. |
+| `-h` | bool | `false` | Display the help dialog and exit. |
+| `-f` | string | `""` | Filter containers by name. |
+| `-a` | bool | `false` | Show active containers only (by default, all containers are shown). |
+| `-s` | string | `""` | Select the container sort field (e.g., `cpu`, `mem`, `name`). |
+| `-r` | bool | `false` | Reverse the container sort order. |
+| `-i` | bool | `false` | Invert the default colors for the UI. |
+| `-connector` | string | `docker` | The container connector to use (e.g., `docker`, `runc`). |
+
+## 4. Examples on How to Use
+
+**Run with default settings (Docker connector, show all containers):**
+```bash
+ctop
+```
+
+**Show only running containers:**
+```bash
+ctop -a
+```
+
+**Filter containers by name (e.g., only show containers with "app" in the name):**
+```bash
+ctop -f app
+```
+
+**Sort containers by CPU usage in descending order:**
+```bash
+ctop -s cpu -r
+```
+
+**Use the runc connector instead of Docker:**
+```bash
+ctop -connector runc
+```

LINTING.md

@@ -0,0 +1,44 @@
+### Code Review Summary
+
+The work was divided into two main tasks, both aimed at improving code quality and correctness by addressing static analysis findings.
+
+---
+
+#### Task 1: Fix `go vet` Issues
+
+*   **Goal:** Address all issues reported by the `go vet ./...` command.
+*   **Files Changed:** `main.go`, `menus.go`.
+*   **Summary of Changes:**
+    1.  **`main.go`:** Removed unreachable `fmt.Printf` and `os.Exit` calls from the `panicExit` function. These lines were placed after a `panic(r)` call, which guarantees they would never be executed.
+    2.  **`menus.go`:** Converted all unkeyed `menu.Item` struct literals (e.g., `menu.Item{"value", "label"}`) to keyed literals (e.g., `menu.Item{Val: "value", Label: "label"}`).
+*   **Accuracy and Completeness:**
+    *   The changes are **accurate**. Removing unreachable code is a standard cleanup, and converting to keyed literals is a Go best practice for readability and maintainability.
+    *   The task was **complete**. After the changes, `go vet ./...` ran successfully with no output, confirming all reported issues were resolved.
+
+---
+
+#### Task 2: Fix `golangci-lint` Issues
+
+*   **Goal:** Address all 51 issues reported by `golangci-lint run ./...` without performing any module updates.
+*   **Files Changed:** Numerous files across the `connector`, `cwidgets`, `config`, `logging`, and `widgets` packages.
+*   **Summary of Changes:**
+    1.  **Error Handling (`errcheck`):** Added error handling for 9 function calls where the error return value was previously ignored. The standard practice applied was to log the error.
+    2.  **Deprecation & Modernization (`govet`, `staticcheck`):**
+        *   Replaced deprecated `// +build` directives with the modern `//go:build` syntax.
+        *   Replaced the deprecated `io/ioutil` package with the `os` package for reading directories.
+        *   Updated deprecated function calls, most notably `rand.Seed`, to use the modern approach of creating a local `rand.New(rand.NewSource(...))` generator.
+        *   Updated deprecated Docker client methods to their current equivalents (e.g., `InspectContainer` to `InspectContainerWithOptions`).
+    3.  **Code Correctness (`staticcheck`):**
+        *   Fixed a bug in `cwidgets/single/hist.go` where the `Append` method for `FloatHist` used a value receiver instead of a pointer receiver, causing state modifications to be lost.
+        *   Fixed an ineffective `break` statement within a `select` block by using a labeled `break` to exit the parent `for` loop correctly.
+    4.  **Unused Code (`unused`):** Removed 14 instances of unused code, including functions, global variables, and struct fields, which cleans the codebase and reduces cognitive overhead.
+    5.  **Code Style & Quality (`staticcheck`):**
+        *   Renamed the `ActionNotImplErr` variable to `ErrActionNotImpl` to conform to Go's error naming conventions.
+        *   Simplified code by replacing `strings.Replace` with `strings.ReplaceAll` where appropriate and removing redundant `break` statements from `switch` cases.
+*   **Accuracy and Completeness:**
+    *   The changes are **accurate**. Each change directly addresses a specific linter warning and adheres to Go best practices. The process was iterative; after fixing the initial set of issues, the linter was re-run multiple times to find and fix any secondary issues (like unused imports or newly created errors) until the codebase was fully clean.
+    *   The task was **complete**. The final run of `golangci-lint run ./...` reported "0 issues," confirming that all findings were successfully and comprehensively addressed.
+
+### Conclusion
+
+The code review confirms that all the requested changes were performed **completely and accurately**. The codebase is now compliant with the stricter `golangci-lint` checks, resulting in improved quality, correctness, and maintainability.

config/main.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"fmt"
-	"os"
 	"sync"
 
 	"github.com/bcicen/ctop/logging"
@@ -35,12 +34,3 @@ func Init() {
 func quote(s string) string {
 	return fmt.Sprintf("\"%s\"", s)
 }
-
-// Return env var value if set, else return defaultVal
-func getEnv(key, defaultVal string) string {
-	val := os.Getenv(key)
-	if val != "" {
-		return val
-	}
-	return defaultVal
-}

connector/collector/docker.go

@@ -37,7 +37,9 @@ func (c *Docker) Start() {
 			Stream: true,
 			Done:   c.done,
 		}
-		c.client.Stats(opts)
+		if err := c.client.Stats(opts); err != nil {
+			log.Errorf("collector failed for container %s: %s", c.id, err)
+		}
 		c.running = false
 	}()
 

connector/collector/mock.go

@@ -1,5 +1,4 @@
 //go:build !release
-// +build !release
 
 package collector
 
@@ -53,23 +52,23 @@ func (c *Mock) Logs() LogCollector {
 
 func (c *Mock) run() {
 	c.running = true
-	rand.Seed(int64(time.Now().Nanosecond()))
+	r := rand.New(rand.NewSource(time.Now().UnixNano()))
 	defer close(c.stream)
 
 	// set to random static value, once
-	c.Pids = rand.Intn(12)
-	c.IOBytesRead = rand.Int63n(8098) * c.aggression
-	c.IOBytesWrite = rand.Int63n(8098) * c.aggression
+	c.Pids = r.Intn(12)
+	c.IOBytesRead = r.Int63n(8098) * c.aggression
+	c.IOBytesWrite = r.Int63n(8098) * c.aggression
 
 	for {
-		c.CPUUtil += rand.Intn(2) * int(c.aggression)
+		c.CPUUtil += r.Intn(2) * int(c.aggression)
 		if c.CPUUtil >= 100 {
 			c.CPUUtil = 0
 		}
 
-		c.NetTx += rand.Int63n(60) * c.aggression
-		c.NetRx += rand.Int63n(60) * c.aggression
-		c.MemUsage += rand.Int63n(c.MemLimit/512) * c.aggression
+		c.NetTx += r.Int63n(60) * c.aggression
+		c.NetRx += r.Int63n(60) * c.aggression
+		c.MemUsage += r.Int63n(c.MemLimit/512) * c.aggression
 		if c.MemUsage > c.MemLimit {
 			c.MemUsage = 0
 		}

connector/collector/mock_logs.go

@@ -15,10 +15,11 @@ type MockLogs struct {
 func (l *MockLogs) Stream() chan models.Log {
 	logCh := make(chan models.Log)
 	go func() {
+	LOOP:
 		for {
 			select {
 			case <-l.done:
-				break
+				break LOOP
 			default:
 				logCh <- models.Log{Timestamp: time.Now(), Message: mockLog}
 				time.Sleep(250 * time.Millisecond)

connector/collector/proc.go

@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package collector
 
@@ -11,7 +10,7 @@ var sysMemTotal = getSysMemTotal()
 
 const (
 	clockTicksPerSecond  uint64 = 100
-	nanoSecondsPerSecond        = 1e9
+	nanoSecondsPerSecond uint64 = 1e9
 )
 
 func getSysMemTotal() int64 {

connector/collector/runc.go

@@ -1,5 +1,4 @@
 //go:build linux
-// +build linux
 
 package collector
 

connector/docker.go

@@ -86,7 +86,9 @@ func (cm *Docker) watchEvents() {
 		"event": {"create", "start", "health_status", "pause", "unpause", "stop", "die", "destroy"},
 	},
 	}
-	cm.client.AddEventListenerWithOptions(opts, events)
+	if err := cm.client.AddEventListenerWithOptions(opts, events); err != nil {
+		log.Errorf("failed to add docker event listener: %s", err)
+	}
 
 	for e := range events {
 		actionName := e.Action
@@ -147,14 +149,12 @@ func webPort(ports map[api.Port][]api.PortBinding) string {
 		if len(v) == 0 {
 			continue
 		}
-		for _, binding := range v {
-			publishedIp := binding.HostIP
-			if publishedIp == "0.0.0.0" {
-				publishedIp = "localhost"
-			}
-			publishedWebPort := fmt.Sprintf("%s:%s", publishedIp, binding.HostPort)
-			return publishedWebPort
+		binding := v[0]
+		publishedIp := binding.HostIP
+		if publishedIp == "0.0.0.0" {
+			publishedIp = "localhost"
 		}
+		return fmt.Sprintf("%s:%s", publishedIp, binding.HostPort)
 	}
 	return ""
 }
@@ -196,7 +196,8 @@ func (cm *Docker) refresh(c *container.Container) {
 }
 
 func (cm *Docker) inspect(id string) (insp *api.Container, found bool, failed bool) {
-	c, err := cm.client.InspectContainer(id)
+	opts := api.InspectContainerOptions{ID: id}
+	c, err := cm.client.InspectContainerWithOptions(opts)
 	if err != nil {
 		if _, notFound := err.(*api.NoSuchContainer); notFound {
 			return c, false, false

connector/main.go

@@ -38,7 +38,7 @@ type ConnectorSuper struct {
 func NewConnectorSuper(connFn ConnectorFn) *ConnectorSuper {
 	cs := &ConnectorSuper{
 		connFn: connFn,
-		err:    fmt.Errorf("connecting..."),
+		err:    fmt.Errorf("connecting"),
 	}
 	go cs.loop()
 	return cs
@@ -79,15 +79,15 @@ func (cs *ConnectorSuper) loop() {
 
 			// wait until connection closed
 			cs.conn.Wait()
-			cs.setError(fmt.Errorf("attempting to reconnect..."))
+			cs.setError(fmt.Errorf("attempting to reconnect"))
 			log.Infof("connector closed")
 		}
 	}
 }
 
 // Enabled returns names for all enabled connectors on the current platform
 func Enabled() (a []string) {
-	for k, _ := range enabled {
+	for k := range enabled {
 		a = append(a, k)
 	}
 	sort.Strings(a)

connector/manager/docker.go

@@ -62,13 +62,10 @@ func (w *frameWriter) Write(p []byte) (n int, err error) {
 		switch p[0] {
 		case STDIN:
 			targetWriter = w.stdin
-			break
 		case STDOUT:
 			targetWriter = w.stdout
-			break
 		case STDERR:
 			targetWriter = w.stderr
-			break
 		default:
 			return 0, wrongFrameFormat
 		}
@@ -103,7 +100,8 @@ func (dc *Docker) Exec(cmd []string) error {
 }
 
 func (dc *Docker) Start() error {
-	c, err := dc.client.InspectContainer(dc.id)
+	opts := api.InspectContainerOptions{ID: dc.id}
+	c, err := dc.client.InspectContainerWithOptions(opts)
 	if err != nil {
 		return fmt.Errorf("cannot inspect container: %v", err)
 	}

connector/manager/main.go

@@ -2,7 +2,7 @@ package manager
 
 import "errors"
 
-var ActionNotImplErr = errors.New("action not implemented")
+var ErrActionNotImpl = errors.New("action not implemented")
 
 type Manager interface {
 	Start() error

connector/manager/mock.go

@@ -7,29 +7,29 @@ func NewMock() *Mock {
 }
 
 func (m *Mock) Start() error {
-	return ActionNotImplErr
+	return ErrActionNotImpl
 }
 
 func (m *Mock) Stop() error {
-	return ActionNotImplErr
+	return ErrActionNotImpl
 }
 
 func (m *Mock) Remove() error {
-	return ActionNotImplErr
+	return ErrActionNotImpl
 }
 
 func (m *Mock) Pause() error {
-	return ActionNotImplErr
+	return ErrActionNotImpl
 }
 
 func (m *Mock) Unpause() error {
-	return ActionNotImplErr
+	return ErrActionNotImpl
 }
 
 func (m *Mock) Restart() error {
-	return ActionNotImplErr
+	return ErrActionNotImpl
 }
 
 func (m *Mock) Exec(cmd []string) error {
-	return ActionNotImplErr
+	return ErrActionNotImpl
 }