diff --git a/AGENTS.md b/AGENTS.md index befa397d..55f28ded 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,7 +8,7 @@ Use `lets` task runner for all build/test/lint operations instead of raw command ```bash lets build [bin] # build CLI with version metadata -lets build-and-install # build and install lets-dev locally +lets build-and-install # build and install lets binary locally lets test # full suite: unit + bats + completions lets test-unit # Go unit tests only lets test-bats [test] # Docker-based Bats integration tests @@ -61,11 +61,6 @@ This is a Single-context repo; skills should read the root Context and root ADRs - `internal/set/` — generic Set data structure - `internal/test/` — test utilities (temp files, args helpers) -## Key lets.yaml Fields - -- Top-level: `shell`, `env`, `before`, `init`, `mixins`, `commands` -- Command: `cmd`, `description`, `depends`, `env`, `options` (docopt), `work_dir`, `after`, `checksum`, `persist_checksum`, `ref`, `args`, `shell` - ## Project Rules - Follow `gofmt` exactly; tabs for indentation, ~120 char lines @@ -73,6 +68,9 @@ This is a Single-context repo; skills should read the root Context and root ADRs - Fixtures in matching `tests//` folder, use `lets.yaml` unless variant needed - Bats tests use `run` + `assert_success`/`assert_line` pattern - Run at least `go test ./...` before considering work complete; `lets test-bats` for CLI-path changes +- Run `lets lint` to verify code quality before commit/push/PR creation +- If you discover non-obvious knowledge needed to make something work or avoid a known issue, document it in code comments or docs so it is not lost +- Add concise code comments for non-obvious logic, invariants, and surprising decisions; do not comment self-explanatory code - **Golden tests** — `internal/cmd/testdata/*` are snapshot of the rendered help and error output. If you change anything that affects help or error rendering (flags, styles, section titles, error messages), regenerate them with `go test ./internal/cmd/ -run -update` (or `lets test-unit --update-golden` in Docker), then commit the updated `.golden` files. If you add a new rendering behaviour (new section, new error type, new command layout), add a corresponding golden test in `internal/cmd/help_golden_test.go` with a fixture YAML in `internal/cmd/testdata/fixtures/` if needed, then run with `-update` to create the golden file. - Commits: short imperative subjects (`Add ...`, `Fix ...`, `Use ...`), explain non-obvious context in body - **Changelog workflow**: add entries to the `Unreleased` section in `docs/docs/changelog.md` with each commit/PR. At release time, rename `Unreleased` to the new tag version diff --git a/CONTEXT.md b/CONTEXT.md index 0405d751..b26ffc89 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -19,8 +19,10 @@ This is a single-context repo. | Term | Definition | Aliases to avoid | | ---- | ---------- | ---------------- | | **Project config** | The repository-level declaration in `lets.yaml` that defines commands and shared execution rules. | Settings, user config | +| **Remote config** | A main Project config loaded from a URL instead of from a local `lets.yaml` file. | Remote root config, URL config | | **Settings** | Per-user lets behavior stored in `~/.config/lets/config.yaml`. | Project config | | **Mixin** | An additional config file merged additively into a Project config. | Override, patch | +| **Remote mixin** | A Mixin loaded from a URL and merged into the main Project config. | Remote config, URL mixin | | **Global env** | Top-level environment entries shared by all Project commands. | Process env | | **Env file** | A dotenv-style file loaded into command execution at global or command scope. | Settings file, config file | | **Theme** | A named style for lets help and styled error output. | Color scheme, palette | @@ -47,6 +49,7 @@ This is a single-context repo. | **Before script** | A top-level script prepended to each Project command invocation, including dependencies. | Init script | | **After script** | A command-scoped script run after a Project command execution attempt. | Cleanup hook | | **Work dir** | The directory where a Project command runs after config and command resolution. | Repo root | +| **Download progress indicator** | A user-visible status shown while lets retrieves a Remote config or Remote mixin. | Progress bar | | **Help surface** | The rendered CLI help for root and Project commands. | Docs page | | **LSP surface** | The editor-facing language-server features exposed by `lets self lsp`. | CLI help | diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 2dbc001d..1d23ac0d 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -5,7 +5,8 @@ title: Changelog ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) -* `[Added]` Remote config support: `lets -c https://url` downloads and caches config to `~/.config/lets/remote-configs/`. Use `--no-cache` to force re-download. Only standalone configs (no `mixins:`) are supported for now. +* `[Added]` Show interactive download progress for remote configs and remote mixins. Issue [#360](https://github.com/lets-cli/lets/issues/360) +* `[Added]` Remote config support: `lets -c https://url` downloads and caches config to `~/.config/lets/remote-configs/`. Use `--no-cache` to force re-download. * `[Added]` Add `lets self skills` commands to show, install, and update the bundled lets agent skill. * `[Docs]` Document the bundled lets Agent Skill and link it from the config reference. * `[Changed]` Expand the bundled lets agent skill with config authoring guidance. diff --git a/docs/docs/config.md b/docs/docs/config.md index e411bc75..58eedcb1 100644 --- a/docs/docs/config.md +++ b/docs/docs/config.md @@ -14,6 +14,7 @@ title: Config reference - [Mixins](#mixins) - [Ignored mixins](#ignored-mixins) - [Remote mixins `(experimental)`](#remote-mixins-experimental) + - [Remote configs](#remote-configs) - [Commands](#commands) - [Command directives:](#command-directives) - [Short syntax](#short-syntax) @@ -326,6 +327,7 @@ Now if `my.yaml` exists - it will be loaded as a mixin. If it is not exist - `le It is possible to specify mixin as url. Lets will download it and load it as a mixin. File will be stored in `.lets/mixins` directory. +When stderr is an interactive terminal, lets shows download progress for remote mixin downloads. Cache hits do not show progress. By default mixin filename will be sha256 hash of url. @@ -341,6 +343,23 @@ mixins: ``` +### Remote configs + +It is possible to load the main lets config from a url with `--config` / `-c`. + +For example: + +```bash +lets -c https://example.com/lets.yaml build +``` + +Lets will download the config and cache it in `~/.config/lets/remote-configs`. +Use `--no-cache` to force lets to re-download the remote config instead of using the cached copy. + +Commands from a remote config run from the directory where `lets` was invoked unless the command specifies `work_dir`. +When stderr is an interactive terminal, lets shows download progress for remote config downloads. Cache hits do not show progress. + + ### Commands `key: commands` diff --git a/go.mod b/go.mod index 97657a1c..f47d252d 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.26 toolchain go1.26.0 require ( + charm.land/bubbles/v2 v2.1.0 charm.land/lipgloss/v2 v2.0.2 github.com/charmbracelet/colorprofile v0.4.2 github.com/charmbracelet/x/ansi v0.11.6 @@ -27,7 +28,6 @@ require ( ) require ( - charm.land/bubbles/v2 v2.1.0 // indirect charm.land/bubbletea/v2 v2.0.2 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/aymanbagabas/go-udiff v0.4.1 // indirect diff --git a/go.sum b/go.sum index 1901af7d..d7d7e604 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,6 @@ charm.land/bubbles/v2 v2.1.0 h1:YSnNh5cPYlYjPxRrzs5VEn3vwhtEn3jVGRBT3M7/I0g= charm.land/bubbles/v2 v2.1.0/go.mod h1:l97h4hym2hvWBVfmJDtrEHHCtkIKeTEb3TTJ4ZOB3wY= charm.land/bubbletea/v2 v2.0.2 h1:4CRtRnuZOdFDTWSff9r8QFt/9+z6Emubz3aDMnf/dx0= charm.land/bubbletea/v2 v2.0.2/go.mod h1:3LRff2U4WIYXy7MTxfbAQ+AdfM3D8Xuvz2wbsOD9OHQ= -charm.land/lipgloss/v2 v2.0.1 h1:6Xzrn49+Py1Um5q/wZG1gWgER2+7dUyZ9XMEufqPSys= -charm.land/lipgloss/v2 v2.0.1/go.mod h1:KjPle2Qd3YmvP1KL5OMHiHysGcNwq6u83MUjYkFvEkM= charm.land/lipgloss/v2 v2.0.2 h1:xFolbF8JdpNkM2cEPTfXEcW1p6NRzOWTSamRfYEw8cs= charm.land/lipgloss/v2 v2.0.2/go.mod h1:KjPle2Qd3YmvP1KL5OMHiHysGcNwq6u83MUjYkFvEkM= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= @@ -106,8 +104,6 @@ github.com/mattn/go-isatty v0.0.0-20160806122752-66b8e73f3f5c/go.mod h1:M+lRXTBq github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/mattn/go-runewidth v0.0.20 h1:WcT52H91ZUAwy8+HUkdM3THM6gXqXuLJi9O3rjcQQaQ= -github.com/mattn/go-runewidth v0.0.20/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/mattn/go-runewidth v0.0.21 h1:jJKAZiQH+2mIinzCJIaIG9Be1+0NR+5sz/lYEEjdM8w= github.com/mattn/go-runewidth v0.0.21/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 0960a59b..c9be4f3e 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -12,8 +12,9 @@ import ( "github.com/lets-cli/fang" "github.com/lets-cli/lets/internal/cmd" - "github.com/lets-cli/lets/internal/config/config" loader "github.com/lets-cli/lets/internal/config" + "github.com/lets-cli/lets/internal/config/config" + "github.com/lets-cli/lets/internal/downloadprogress" "github.com/lets-cli/lets/internal/env" "github.com/lets-cli/lets/internal/logging" "github.com/lets-cli/lets/internal/set" @@ -79,15 +80,26 @@ func Main(version string, buildDate string) int { } var cfg *config.Config + + loadOptions := []loader.LoadOption{} + if isInteractiveStderr() { + loadOptions = append(loadOptions, loader.WithProgress(downloadprogress.New( + os.Stderr, + downloadprogress.WithNoColor(appSettings.NoColor), + downloadprogress.WithTheme(appSettings.Theme), + ))) + } + if isRemoteURL(rootFlags.config) { if configDir != "" { log.Warnf("LETS_CONFIG_DIR is ignored when using a remote config URL") } - cfg, err = loader.LoadRemote(ctx, rootFlags.config, rootFlags.noCache, version) + cfg, err = loader.LoadRemote(ctx, rootFlags.config, rootFlags.noCache, version, loadOptions...) } else { - cfg, err = loader.Load(rootFlags.config, configDir, version) + cfg, err = loader.LoadWithContext(ctx, rootFlags.config, configDir, version, loadOptions...) } + if err != nil { if failOnConfigError(rootCmd, command, rootFlags) { log.Errorf("config error: %s", err) diff --git a/internal/cmd/self_skills.go b/internal/cmd/self_skills.go index 2adf4ec3..ebaf75b7 100644 --- a/internal/cmd/self_skills.go +++ b/internal/cmd/self_skills.go @@ -136,7 +136,7 @@ func promptSkillScope(in io.Reader, out io.Writer, localDir string, globalDir st _, _ = fmt.Fprint(out, "Select [1/2]: ") line, err := bufio.NewReader(in).ReadString('\n') - if err != nil && !(errors.Is(err, io.EOF) && line != "") { + if err != nil && (!errors.Is(err, io.EOF) || line == "") { return "", fmt.Errorf("reading install scope: %w", err) } @@ -180,6 +180,7 @@ func validateSkillNameArg(args []string) error { func installLetsSkill(out io.Writer, targetDir string, force bool) error { skillDir := filepath.Join(targetDir, skillpkg.LetsName) + skillPath := filepath.Join(skillDir, skillpkg.SkillFile) if _, err := os.Stat(skillPath); err == nil && !force { _, _ = fmt.Fprintf(out, "%s already exists. Use --force to overwrite.\n", skillPath) @@ -193,6 +194,7 @@ func installLetsSkill(out io.Writer, targetDir string, force bool) error { } _, _ = fmt.Fprintf(out, "Installed %s\n", skillDir) + return nil } @@ -203,12 +205,15 @@ func updateLetsSkill(out io.Writer) error { } updated := 0 + for _, skillDir := range dirs { skillPath := filepath.Join(skillDir, skillpkg.SkillFile) + current, err := os.ReadFile(skillPath) if errors.Is(err, os.ErrNotExist) { continue } + if err != nil { return fmt.Errorf("reading %s: %w", skillPath, err) } @@ -216,6 +221,7 @@ func updateLetsSkill(out io.Writer) error { if bytes.Equal(current, skillpkg.LetsSkill()) { _, _ = fmt.Fprintf(out, "%s already up to date.\n", skillDir) updated++ + continue } @@ -257,6 +263,7 @@ func knownLetsSkillDirs() ([]string, error) { if err != nil { return nil, err } + dirs = append(dirs, filepath.Join(globalDir, skillpkg.LetsName)) return dirs, nil @@ -302,6 +309,7 @@ func findGitRoot(start string) (string, error) { if parent == dir { return "", errors.New("not in a Git repository. Use --global or --path to specify a target") } + dir = parent } } diff --git a/internal/config/config/config.go b/internal/config/config/config.go index ad014d3f..1cfd0cd0 100644 --- a/internal/config/config/config.go +++ b/internal/config/config/config.go @@ -2,6 +2,7 @@ package config import ( "bytes" + "context" "errors" "fmt" "maps" @@ -10,6 +11,7 @@ import ( "strings" "github.com/lets-cli/lets/internal/config/path" + "github.com/lets-cli/lets/internal/fetch" "github.com/lets-cli/lets/internal/set" "github.com/lets-cli/lets/internal/util" "gopkg.in/yaml.v3" @@ -52,8 +54,23 @@ type Config struct { RemoteSource string // cached env after config.SetupEnv, used in config.GetEnv - cachedEnv map[string]string - isMixin bool // if true, we consider config as mixin and apply different parsing and validation + cachedEnv map[string]string + downloadContext context.Context + downloadProgress fetch.ProgressObserver + isMixin bool // if true, we consider config as mixin and apply different parsing and validation +} + +func (c *Config) SetDownloadOptions(ctx context.Context, progress fetch.ProgressObserver) { + c.downloadContext = ctx + c.downloadProgress = progress +} + +func (c *Config) context() context.Context { + if c.downloadContext == nil { + return context.Background() + } + + return c.downloadContext } func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { @@ -225,7 +242,7 @@ func (c *Config) readMixin(mixin *Mixin) error { } if data == nil { - data, err = rm.download() + data, err = rm.download(c.context(), c.downloadProgress) if err != nil { return err } @@ -356,6 +373,7 @@ func NewConfig(workDir string, configAbsPath string, dotLetsDir string) *Config func NewMixinConfig(cfg *Config, configAbsPath string) *Config { mixin := NewConfig(cfg.WorkDir, configAbsPath, cfg.DotLetsDir) mixin.isMixin = true + mixin.SetDownloadOptions(cfg.context(), cfg.downloadProgress) return mixin } diff --git a/internal/config/config/mixin.go b/internal/config/config/mixin.go index 2ce5a5ad..77e5fc8e 100644 --- a/internal/config/config/mixin.go +++ b/internal/config/config/mixin.go @@ -77,8 +77,8 @@ func (rm *RemoteMixin) tryRead() ([]byte, error) { return data, nil } -func (rm *RemoteMixin) download() ([]byte, error) { - return fetch.Download(context.Background(), rm.URL) +func (rm *RemoteMixin) download(ctx context.Context, progress fetch.ProgressObserver) ([]byte, error) { + return fetch.Download(ctx, rm.URL, fetch.WithProgress(fetch.SourceRemoteMixin, progress)) } // Trim `-` prefix. diff --git a/internal/config/load.go b/internal/config/load.go index 771d5cc7..baf87851 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -16,19 +16,48 @@ import ( "gopkg.in/yaml.v3" ) +type loadOptions struct { + progress fetch.ProgressObserver +} + +type LoadOption func(*loadOptions) + +func WithProgress(progress fetch.ProgressObserver) LoadOption { + return func(opts *loadOptions) { + opts.progress = progress + } +} + +func newLoadOptions(options []LoadOption) loadOptions { + opts := loadOptions{} + for _, option := range options { + option(&opts) + } + + return opts +} + func Load(configName string, configDir string, version string) (*config.Config, error) { + return LoadWithContext(context.Background(), configName, configDir, version) +} + +func LoadWithContext(ctx context.Context, configName string, configDir string, version string, options ...LoadOption) (*config.Config, error) { + opts := newLoadOptions(options) + configPath, err := FindConfig(configName, configDir) if err != nil { return nil, err } - return loadConfigFromFile(configPath.AbsPath, configPath.WorkDir, configPath.DotLetsDir, configPath.Filename, version) + return loadConfigFromFile(ctx, configPath.AbsPath, configPath.WorkDir, configPath.DotLetsDir, configPath.Filename, version, opts) } // LoadRemote downloads (or loads from cache) a remote lets.yaml at url and // returns a Config with the working directory set to the caller's CWD. -func LoadRemote(ctx context.Context, url string, noCache bool, version string) (*config.Config, error) { - cachedPath, err := ensureRemoteConfig(ctx, url, noCache) +func LoadRemote(ctx context.Context, url string, noCache bool, version string, options ...LoadOption) (*config.Config, error) { + opts := newLoadOptions(options) + + cachedPath, err := ensureRemoteConfig(ctx, url, noCache, opts.progress) if err != nil { return nil, err } @@ -47,7 +76,7 @@ func LoadRemote(ctx context.Context, url string, noCache bool, version string) ( return nil, fmt.Errorf("can not create .lets dir: %w", err) } - c, err := loadConfigFromFile(cachedPath, cwd, dotLetsDir, url, version) + c, err := loadConfigFromFile(ctx, cachedPath, cwd, dotLetsDir, url, version, opts) if err != nil { return nil, fmt.Errorf("%w (use --no-cache to re-download)", err) } @@ -59,7 +88,11 @@ func LoadRemote(ctx context.Context, url string, noCache bool, version string) ( // loadConfigFromFile is shared by Load and LoadRemote: opens the file at absPath, // decodes YAML, validates, and sets up env. displayName appears in parse error messages. -func loadConfigFromFile(absPath, workDir, dotLetsDir, displayName, version string) (*config.Config, error) { +func loadConfigFromFile( + ctx context.Context, + absPath, workDir, dotLetsDir, displayName, version string, + opts loadOptions, +) (*config.Config, error) { f, err := os.Open(absPath) if err != nil { return nil, err @@ -67,6 +100,8 @@ func loadConfigFromFile(absPath, workDir, dotLetsDir, displayName, version strin defer f.Close() c := config.NewConfig(workDir, absPath, dotLetsDir) + c.SetDownloadOptions(ctx, opts.progress) + if err := yaml.NewDecoder(f).Decode(c); err != nil { return nil, fmt.Errorf("failed to parse %s: %w", displayName, err) } @@ -82,7 +117,7 @@ func loadConfigFromFile(absPath, workDir, dotLetsDir, displayName, version strin return c, nil } -func ensureRemoteConfig(ctx context.Context, url string, noCache bool) (string, error) { +func ensureRemoteConfig(ctx context.Context, url string, noCache bool, progress fetch.ProgressObserver) (string, error) { cacheDir, err := remoteConfigCacheDir() if err != nil { return "", err @@ -98,7 +133,7 @@ func ensureRemoteConfig(ctx context.Context, url string, noCache bool) (string, return cachePath, nil } - data, downloadErr := fetch.Download(ctx, url) + data, downloadErr := fetch.Download(ctx, url, fetch.WithProgress(fetch.SourceRemoteConfig, progress)) if downloadErr != nil { if util.FileExists(cachePath) { log.Warnf("failed to download remote config (%v), falling back to cached version", downloadErr) @@ -132,6 +167,7 @@ func writeCacheAtomic(dst string, data []byte) error { if writeErr != nil || closeErr != nil { os.Remove(tmpPath) + if writeErr != nil { return fmt.Errorf("failed to write temp cache file: %w", writeErr) } diff --git a/internal/config/load_test.go b/internal/config/load_test.go index da2df7ed..978c8432 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -8,8 +8,25 @@ import ( "path/filepath" "strings" "testing" + + "github.com/lets-cli/lets/internal/fetch" ) +type recordingProgress struct { + starts []fetch.ProgressInfo +} + +func (p *recordingProgress) Start(info fetch.ProgressInfo) fetch.ProgressTracker { + p.starts = append(p.starts, info) + return noopTracker{} +} + +type noopTracker struct{} + +func (noopTracker) Add(int64) {} + +func (noopTracker) Done(error) {} + func TestLoadConfig(t *testing.T) { t.Run("just load config", func(t *testing.T) { _, err := Load("", "", "0.0.0-test") @@ -92,12 +109,38 @@ func TestLoadRemote(t *testing.T) { if _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test"); err != nil { t.Fatalf("first call error: %v", err) } - if _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test"); err != nil { + progress := &recordingProgress{} + if _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test", WithProgress(progress)); err != nil { t.Fatalf("second call error: %v", err) } if requests != 1 { t.Fatalf("expected 1 HTTP request, got %d", requests) } + if len(progress.starts) != 0 { + t.Fatalf("expected no progress starts for cache hit, got %d", len(progress.starts)) + } + }) + + t.Run("reports progress for remote config download", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(validConfig)) + })) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Chdir(t.TempDir()) + + progress := &recordingProgress{} + if _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test", WithProgress(progress)); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(progress.starts) != 1 { + t.Fatalf("expected 1 progress start, got %d", len(progress.starts)) + } + if progress.starts[0].Kind != fetch.SourceRemoteConfig { + t.Fatalf("expected remote config progress, got %q", progress.starts[0].Kind) + } }) t.Run("re-downloads with --no-cache", func(t *testing.T) { @@ -162,4 +205,38 @@ func TestLoadRemote(t *testing.T) { t.Fatal("expected error when no cache and download fails") } }) + + t.Run("reports progress for remote mixin cache miss only", func(t *testing.T) { + mixinConfig := "commands:\n mixed:\n cmd: echo mixed\n" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(mixinConfig)) + })) + defer srv.Close() + + tempDir := t.TempDir() + mainConfig := "shell: bash\nmixins:\n - url: " + srv.URL + "\ncommands:\n ok:\n cmd: echo ok\n" + if err := os.WriteFile(filepath.Join(tempDir, "lets.yaml"), []byte(mainConfig), 0o644); err != nil { + t.Fatalf("write config: %v", err) + } + + progress := &recordingProgress{} + if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test", WithProgress(progress)); err != nil { + t.Fatalf("first load error: %v", err) + } + if len(progress.starts) != 1 { + t.Fatalf("expected 1 progress start, got %d", len(progress.starts)) + } + if progress.starts[0].Kind != fetch.SourceRemoteMixin { + t.Fatalf("expected remote mixin progress, got %q", progress.starts[0].Kind) + } + + cacheHitProgress := &recordingProgress{} + if _, err := LoadWithContext(ctx, "", tempDir, "0.0.0-test", WithProgress(cacheHitProgress)); err != nil { + t.Fatalf("second load error: %v", err) + } + if len(cacheHitProgress.starts) != 0 { + t.Fatalf("expected no progress starts for remote mixin cache hit, got %d", len(cacheHitProgress.starts)) + } + }) } diff --git a/internal/downloadprogress/progress.go b/internal/downloadprogress/progress.go new file mode 100644 index 00000000..9d1360b1 --- /dev/null +++ b/internal/downloadprogress/progress.go @@ -0,0 +1,587 @@ +package downloadprogress + +import ( + "fmt" + "image/color" + "io" + "math" + "net/url" + "path" + "strings" + "sync" + "time" + + bubblesprogress "charm.land/bubbles/v2/progress" + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/term" + "github.com/lets-cli/lets/internal/fetch" + "github.com/lets-cli/lets/internal/theme" +) + +const ( + defaultWidth = 80 + minBarWidth = 10 + maxBarWidth = 30 +) + +type Observer struct { + writer io.Writer + width int + noColor bool + animate bool + throttle time.Duration + finalPause time.Duration + fillColor color.Color + emptyColor color.Color + now func() time.Time +} + +type Option func(*Observer) + +func WithWidth(width int) Option { + return func(observer *Observer) { + observer.width = width + } +} + +func WithNoColor(noColor bool) Option { + return func(observer *Observer) { + observer.noColor = noColor + } +} + +func WithTheme(themeName string) Option { + return func(observer *Observer) { + if observer.noColor { + return + } + + file, ok := observer.writer.(term.File) + if !ok || !term.IsTerminal(file.Fd()) { + return + } + + observer.fillColor, observer.emptyColor = theme.ProgressColorsByName(themeName, file) + } +} + +func WithThrottle(throttle time.Duration) Option { + return func(observer *Observer) { + observer.throttle = throttle + } +} + +func WithFinalPause(finalPause time.Duration) Option { + return func(observer *Observer) { + observer.finalPause = finalPause + } +} + +func WithNow(now func() time.Time) Option { + return func(observer *Observer) { + observer.now = now + } +} + +func New(writer io.Writer, options ...Option) *Observer { + observer := &Observer{ + writer: writer, + width: detectWidth(writer), + throttle: 100 * time.Millisecond, + finalPause: 750 * time.Millisecond, + animate: isTerminal(writer), + now: time.Now, + } + + for _, option := range options { + option(observer) + } + + if observer.width <= 0 { + observer.width = defaultWidth + } + + if observer.now == nil { + observer.now = time.Now + } + + return observer +} + +func (o *Observer) Start(info fetch.ProgressInfo) fetch.ProgressTracker { //nolint:ireturn // Implements fetch.ProgressObserver. + if info.TotalBytes > 0 && o.animate { + return newAnimatedTracker(o, info) + } + + tracker := &manualTracker{ + observer: o, + info: info, + label: downloadLabel(info.URL), + } + tracker.render(false) + + return tracker +} + +type manualTracker struct { + observer *Observer + info fetch.ProgressInfo + label string + read int64 + started bool + lastRender time.Time + lastWidth int +} + +func (t *manualTracker) Add(n int64) { + t.read += n + + now := t.observer.now() + if t.observer.throttle > 0 && !t.lastRender.IsZero() && now.Sub(t.lastRender) < t.observer.throttle { + return + } + + t.render(false) +} + +func (t *manualTracker) Done(err error) { + if err != nil { + _, _ = fmt.Fprintln(t.observer.writer) + return + } + + t.render(true) +} + +func (t *manualTracker) render(done bool) { + if !t.started { + _, _ = fmt.Fprintln(t.observer.writer, t.labelLine()) + t.started = true + } + + line := t.progressLine() + if done { + _, _ = fmt.Fprintf(t.observer.writer, "\r\033[2K%s\n", line) + t.lastWidth = 0 + t.lastRender = t.observer.now() + + return + } + + lineWidth := lipgloss.Width(line) + + padding := "" + if t.lastWidth > lineWidth { + padding = strings.Repeat(" ", t.lastWidth-lineWidth) + } + + _, _ = fmt.Fprintf(t.observer.writer, "\r%s%s", line, padding) + + t.lastWidth = lineWidth + t.lastRender = t.observer.now() +} + +func (t *manualTracker) labelLine() string { + return labelLine("Downloading", t.label, t.observer.width) +} + +func (t *manualTracker) progressLine() string { + if t.info.TotalBytes > 0 { + return t.knownSizeLine() + } + + return t.unknownSizeLine() +} + +func (t *manualTracker) knownSizeLine() string { + percent := clamp(float64(t.read)/float64(t.info.TotalBytes), 0, 1) + suffix := fmt.Sprintf("%3.0f%% %s/%s", percent*100, formatBytes(t.read), formatBytes(t.info.TotalBytes)) + + bar := "" + + barWidth := t.barWidth(suffix) + if barWidth >= minBarWidth { + model := t.progressModel(barWidth) + bar = model.ViewAs(percent) + } + + if bar == "" { + return suffix + } + + return fmt.Sprintf("%s %s", bar, suffix) +} + +func (t *manualTracker) unknownSizeLine() string { + return formatBytes(t.read) +} + +func (t *manualTracker) barWidth(suffix string) int { + spaceForBar := t.observer.width - 1 - lipgloss.Width(suffix) + if spaceForBar < minBarWidth { + return 0 + } + + return min(maxBarWidth, max(minBarWidth, spaceForBar)) +} + +func (t *manualTracker) progressModel(width int) bubblesprogress.Model { + model := bubblesprogress.New( + bubblesprogress.WithWidth(width), + bubblesprogress.WithoutPercentage(), + bubblesprogress.WithFillCharacters('#', '-'), + ) + if t.observer.noColor { + model.FullColor = nil + model.EmptyColor = nil + } else { + applyProgressColors(&model, t.observer.fillColor, t.observer.emptyColor) + } + + return model +} + +type animatedTracker struct { + observer *Observer + program *tea.Program + done chan struct{} + label string + read int64 + total int64 + lastUpdate time.Time +} + +func newAnimatedTracker(observer *Observer, info fetch.ProgressInfo) *animatedTracker { + ready := make(chan struct{}) + done := make(chan struct{}) + label := downloadLabel(info.URL) + _, _ = fmt.Fprintln(observer.writer, labelLine("Downloading", label, observer.width)) + model := newProgressModel(observer, label, info.TotalBytes, ready) + program := tea.NewProgram( + model, + tea.WithInput(nil), + tea.WithOutput(observer.writer), + tea.WithoutSignals(), + ) + + tracker := &animatedTracker{ + observer: observer, + program: program, + done: done, + label: label, + total: info.TotalBytes, + } + + go func() { + _, _ = program.Run() + + close(done) + }() + + <-ready + + return tracker +} + +func (t *animatedTracker) Add(n int64) { + t.read += n + + now := t.observer.now() + if t.observer.throttle > 0 && !t.lastUpdate.IsZero() && now.Sub(t.lastUpdate) < t.observer.throttle { + return + } + + t.program.Send(progressMsg{read: t.read, total: t.total}) + t.lastUpdate = now +} + +func (t *animatedTracker) Done(err error) { + if err != nil { + t.program.Send(progressErrMsg{}) + } else { + t.program.Send(progressDoneMsg{read: t.read, total: t.total}) + } + + <-t.done + + if err == nil { + _, _ = fmt.Fprintf(t.observer.writer, "%s\n", t.progressLine()) + } +} + +func (t *animatedTracker) progressLine() string { + bar := t.progressModel().ViewAs(1) + return fmt.Sprintf("%s 100%% %s/%s", bar, formatBytes(t.total), formatBytes(t.total)) +} + +func (t *animatedTracker) progressModel() bubblesprogress.Model { + model := bubblesprogress.New( + bubblesprogress.WithWidth(barWidthForTerminal(t.observer.width)), + bubblesprogress.WithoutPercentage(), + bubblesprogress.WithFillCharacters('#', '-'), + ) + if t.observer.noColor { + model.FullColor = nil + model.EmptyColor = nil + } else { + applyProgressColors(&model, t.observer.fillColor, t.observer.emptyColor) + } + + return model +} + +type progressMsg struct { + read int64 + total int64 +} + +type progressDoneMsg struct { + read int64 + total int64 +} + +type progressErrMsg struct{} + +type progressQuitMsg struct{} + +type progressModel struct { + label string + read int64 + total int64 + width int + finalPause time.Duration + ready chan struct{} + readyOnce *sync.Once + progress bubblesprogress.Model +} + +func newProgressModel(observer *Observer, label string, total int64, ready chan struct{}) progressModel { + model := bubblesprogress.New( + bubblesprogress.WithWidth(barWidthForTerminal(observer.width)), + bubblesprogress.WithoutPercentage(), + bubblesprogress.WithFillCharacters('#', '-'), + ) + if observer.noColor { + model.FullColor = nil + model.EmptyColor = nil + } else { + applyProgressColors(&model, observer.fillColor, observer.emptyColor) + } + + return progressModel{ + label: label, + total: total, + width: observer.width, + finalPause: observer.finalPause, + ready: ready, + readyOnce: &sync.Once{}, + progress: model, + } +} + +func (m progressModel) Init() tea.Cmd { + m.readyOnce.Do(func() { + close(m.ready) + }) + + return nil +} + +func (m progressModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { //nolint:ireturn // Required by Bubble Tea's model interface. + switch msg := msg.(type) { + case tea.WindowSizeMsg: + m.width = msg.Width + m.progress.SetWidth(barWidthForTerminal(msg.Width)) + + return m, nil + + case progressMsg: + m.read = msg.read + m.total = msg.total + + return m, m.progress.SetPercent(m.percent()) + + case progressDoneMsg: + m.read = msg.read + m.total = msg.total + + return m, tea.Batch(m.progress.SetPercent(1), m.quitAfterFinalPause()) + + case progressErrMsg: + return m, tea.Quit + + case progressQuitMsg: + return m, tea.Quit + + case bubblesprogress.FrameMsg: + var cmd tea.Cmd + + m.progress, cmd = m.progress.Update(msg) + + return m, cmd + + default: + return m, nil + } +} + +func (m progressModel) View() tea.View { + return tea.NewView(m.progressLine()) +} + +func (m progressModel) progressLine() string { + return fmt.Sprintf("%s %3.0f%% %s/%s", m.progress.View(), m.percent()*100, formatBytes(m.read), formatBytes(m.total)) +} + +func (m progressModel) percent() float64 { + if m.total <= 0 { + return 0 + } + + return clamp(float64(m.read)/float64(m.total), 0, 1) +} + +func (m progressModel) quitAfterFinalPause() tea.Cmd { + return tea.Tick(m.finalPause, func(time.Time) tea.Msg { + return progressQuitMsg{} + }) +} + +func barWidthForTerminal(width int) int { + suffixWidth := lipgloss.Width(" 100% 1023.9 KiB/1023.9 KiB") + + spaceForBar := width - 1 - suffixWidth + if spaceForBar < minBarWidth { + return minBarWidth + } + + return min(maxBarWidth, max(minBarWidth, spaceForBar)) +} + +func detectWidth(writer io.Writer) int { + file, ok := writer.(term.File) + if !ok || !isTerminal(writer) { + return defaultWidth + } + + width, _, err := term.GetSize(file.Fd()) + if err != nil || width <= 0 { + return defaultWidth + } + + return width +} + +func isTerminal(writer io.Writer) bool { + file, ok := writer.(term.File) + return ok && term.IsTerminal(file.Fd()) +} + +func applyProgressColors(model *bubblesprogress.Model, fill, empty color.Color) { + if fill != nil { + model.FullColor = fill + } + + if empty != nil { + model.EmptyColor = empty + } +} + +func labelLine(verb, label string, width int) string { + return fmt.Sprintf("%s %s", verb, truncateMiddle(label, width-lipgloss.Width(verb)-1)) +} + +func downloadLabel(rawURL string) string { + parsed, err := url.Parse(rawURL) + if err != nil { + return path.Base(stripSecretParts(rawURL)) + } + + filename := path.Base(parsed.Path) + if filename != "." && filename != "/" && filename != "" { + return filename + } + + if parsed.Host != "" { + return parsed.Host + } + + return redactURL(rawURL) +} + +func redactURL(rawURL string) string { + parsed, err := url.Parse(rawURL) + if err != nil { + return stripSecretParts(rawURL) + } + + parsed.User = nil + parsed.RawQuery = "" + parsed.Fragment = "" + + return parsed.String() +} + +func stripSecretParts(rawURL string) string { + idx := len(rawURL) + if queryIdx := strings.Index(rawURL, "?"); queryIdx >= 0 && queryIdx < idx { + idx = queryIdx + } + + if fragmentIdx := strings.Index(rawURL, "#"); fragmentIdx >= 0 && fragmentIdx < idx { + idx = fragmentIdx + } + + return rawURL[:idx] +} + +func truncateMiddle(s string, width int) string { + if width <= 0 { + return "" + } + + if lipgloss.Width(s) <= width { + return s + } + + if width <= 3 { + return strings.Repeat(".", width) + } + + runes := []rune(s) + keep := width - 3 + left := keep / 2 + right := keep - left + + if left+right >= len(runes) { + return s + } + + return string(runes[:left]) + "..." + string(runes[len(runes)-right:]) +} + +func formatBytes(bytes int64) string { + if bytes < 1024 { + return fmt.Sprintf("%d B", bytes) + } + + units := []string{"KiB", "MiB", "GiB", "TiB"} + value := float64(bytes) / 1024 + + unit := units[0] + for _, nextUnit := range units[1:] { + if value < 1024 { + break + } + + value /= 1024 + unit = nextUnit + } + + return fmt.Sprintf("%.1f %s", value, unit) +} + +func clamp(value, minValue, maxValue float64) float64 { + return math.Max(minValue, math.Min(maxValue, value)) +} diff --git a/internal/downloadprogress/progress_test.go b/internal/downloadprogress/progress_test.go new file mode 100644 index 00000000..e0b16ff6 --- /dev/null +++ b/internal/downloadprogress/progress_test.go @@ -0,0 +1,132 @@ +package downloadprogress + +import ( + "bytes" + "strings" + "testing" + "time" + + "github.com/lets-cli/lets/internal/fetch" +) + +func TestObserver(t *testing.T) { + t.Run("renders unchanged downloading label for known size", func(t *testing.T) { + var out bytes.Buffer + observer := New(&out, WithWidth(120), WithNoColor(true), WithThrottle(0), WithFinalPause(0)) + + tracker := observer.Start(fetch.ProgressInfo{ + Kind: fetch.SourceRemoteConfig, + URL: "https://user:pass@example.com/path/lets.yaml?token=secret#fragment", + TotalBytes: 4, + }) + tracker.Add(4) + tracker.Done(nil) + + got := out.String() + if !strings.Contains(got, "Downloading lets.yaml") || + !strings.Contains(got, "\n\r") || + !strings.Contains(got, "100% 4 B/4 B") { + t.Fatalf("expected completed line, got %q", got) + } + if strings.Contains(got, "Downloaded") { + t.Fatalf("did not expect label to change to Downloaded, got %q", got) + } + if !strings.Contains(got, "#") { + t.Fatalf("expected completed line to include progress bar, got %q", got) + } + if strings.Contains(got, "secret") || strings.Contains(got, "user:pass") || strings.Contains(got, "fragment") { + t.Fatalf("expected URL secrets to be redacted, got %q", got) + } + }) + + t.Run("renders unknown size without percent", func(t *testing.T) { + var out bytes.Buffer + observer := New(&out, WithWidth(120), WithNoColor(true), WithThrottle(0)) + + tracker := observer.Start(fetch.ProgressInfo{ + Kind: fetch.SourceRemoteMixin, + URL: "https://example.com/mixin.yaml", + TotalBytes: -1, + }) + tracker.Add(1536) + tracker.Done(nil) + + got := out.String() + if !strings.Contains(got, "Downloading mixin.yaml") || + !strings.Contains(got, "\n\r") || + !strings.Contains(got, "1.5 KiB") { + t.Fatalf("expected unknown-size completed line, got %q", got) + } + if strings.Contains(got, "Downloaded") { + t.Fatalf("did not expect label to change to Downloaded, got %q", got) + } + if strings.Contains(got, "%") { + t.Fatalf("did not expect percent for unknown size, got %q", got) + } + }) + + t.Run("throttles intermediate renders", func(t *testing.T) { + var out bytes.Buffer + now := time.Unix(0, 0) + observer := New(&out, WithWidth(120), WithNoColor(true), WithNow(func() time.Time { return now })) + + tracker := observer.Start(fetch.ProgressInfo{ + Kind: fetch.SourceRemoteConfig, + URL: "https://example.com/lets.yaml", + TotalBytes: -1, + }) + startOutput := out.String() + tracker.Add(1) + if out.String() != startOutput { + t.Fatalf("expected add before throttle to skip render") + } + + now = now.Add(100 * time.Millisecond) + tracker.Add(1) + if out.String() == startOutput { + t.Fatalf("expected add at throttle boundary to render") + } + }) +} + +func TestFormatBytes(t *testing.T) { + tests := map[int64]string{ + 42: "42 B", + 1024: "1.0 KiB", + 1536: "1.5 KiB", + 1024 * 1024: "1.0 MiB", + } + + for input, want := range tests { + if got := formatBytes(input); got != want { + t.Fatalf("formatBytes(%d) = %q, want %q", input, got, want) + } + } +} + +func TestDownloadLabel(t *testing.T) { + got := downloadLabel("https://user:pass@example.com/path/lets.yaml?token=secret#fragment") + if got != "lets.yaml" { + t.Fatalf("expected filename label, got %q", got) + } +} + +func TestProgressModel(t *testing.T) { + observer := New(&bytes.Buffer{}, WithWidth(120), WithNoColor(true), WithFinalPause(0)) + model := newProgressModel(observer, "lets.yaml", 1107, make(chan struct{})) + + _, cmd := model.Update(progressMsg{read: 512, total: 1107}) + if cmd == nil { + t.Fatal("expected progress update to return animation command") + } + + updated, _ := model.Update(progressDoneMsg{read: 1107, total: 1107}) + finalModel := updated.(progressModel) + got := finalModel.View().Content + if strings.Contains(got, "Downloaded") { + t.Fatalf("did not expect label to change to Downloaded, got %q", got) + } + if !strings.Contains(got, "100% 1.1 KiB/1.1 KiB") { + t.Fatalf("expected final progress status, got %q", got) + } +} diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go index dde85fb8..9110495d 100644 --- a/internal/fetch/fetch.go +++ b/internal/fetch/fetch.go @@ -1,6 +1,7 @@ package fetch import ( + "bytes" "context" "fmt" "io" @@ -19,15 +20,71 @@ var allowedContentTypes = set.NewSet( "application/x-yaml", ) -// httpClient backstop timeout guards against hung connections when callers pass context.Background(). -// 5 minutes matches the previous per-request context timeout used by RemoteMixin downloads. -var httpClient = &http.Client{ - Timeout: 5 * 60 * time.Second, +var httpClient = newHTTPClient() + +func newHTTPClient() *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + // Keep Content-Length usable for known-size download progress bars. + transport.DisableCompression = true + + // Timeout guards against hung connections when callers pass context.Background(). + // 5 minutes matches the previous per-request context timeout used by RemoteMixin downloads. + return &http.Client{ + Timeout: 5 * 60 * time.Second, + Transport: transport, + } +} + +type SourceKind string + +const ( + SourceRemoteConfig SourceKind = "remote config" + SourceRemoteMixin SourceKind = "remote mixin" +) + +type ProgressInfo struct { + Kind SourceKind + URL string + TotalBytes int64 +} + +type ProgressObserver interface { + Start(info ProgressInfo) ProgressTracker +} + +type ProgressTracker interface { + Add(n int64) + Done(err error) +} + +type downloadOptions struct { + progress ProgressObserver + kind SourceKind +} + +type Option func(*downloadOptions) + +func WithProgress(kind SourceKind, progress ProgressObserver) Option { + return func(opts *downloadOptions) { + opts.kind = kind + opts.progress = progress + } +} + +func newDownloadOptions(options []Option) downloadOptions { + opts := downloadOptions{} + for _, option := range options { + option(&opts) + } + + return opts } // Download fetches the content at url, validates the Content-Type is a YAML variant, // and returns the raw bytes. -func Download(ctx context.Context, url string) ([]byte, error) { +func Download(ctx context.Context, url string, options ...Option) ([]byte, error) { + opts := newDownloadOptions(options) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, err @@ -42,11 +99,13 @@ func Download(ctx context.Context, url string) ([]byte, error) { if resp.StatusCode == http.StatusNotFound { return nil, fmt.Errorf("no such file at: %s", url) } + if resp.StatusCode < 200 || resp.StatusCode > 299 { return nil, fmt.Errorf("network error for %s: %s", url, resp.Status) } contentType := resp.Header.Get("Content-Type") + mediaType, _, parseErr := mime.ParseMediaType(contentType) if parseErr != nil { mediaType = contentType @@ -56,10 +115,53 @@ func Download(ctx context.Context, url string) ([]byte, error) { return nil, fmt.Errorf("unsupported content type: %s", contentType) } - data, err := io.ReadAll(resp.Body) + var tracker ProgressTracker + if opts.progress != nil { + tracker = opts.progress.Start(ProgressInfo{ + Kind: opts.kind, + URL: url, + TotalBytes: resp.ContentLength, + }) + } + + data, err := readAll(resp.Body, tracker, resp.ContentLength) + if tracker != nil { + tracker.Done(err) + } + if err != nil { return nil, fmt.Errorf("failed to read response: %w", err) } return data, nil } + +func readAll(reader io.Reader, tracker ProgressTracker, contentLength int64) ([]byte, error) { + var buf bytes.Buffer + if contentLength > 0 && contentLength <= int64(^uint(0)>>1) { + buf.Grow(int(contentLength)) + } + + if tracker == nil { + _, err := io.Copy(&buf, reader) + return buf.Bytes(), err + } + + _, err := io.Copy(&buf, progressReader{reader: reader, tracker: tracker}) + + return buf.Bytes(), err +} + +type progressReader struct { + reader io.Reader + tracker ProgressTracker +} + +func (r progressReader) Read(p []byte) (int, error) { + n, err := r.reader.Read(p) + if n > 0 { + r.tracker.Add(int64(n)) + } + + return n, err +} diff --git a/internal/fetch/fetch_test.go b/internal/fetch/fetch_test.go index ac104a53..3da0f255 100644 --- a/internal/fetch/fetch_test.go +++ b/internal/fetch/fetch_test.go @@ -3,12 +3,36 @@ package fetch_test import ( "net/http" "net/http/httptest" + "strconv" "strings" "testing" "github.com/lets-cli/lets/internal/fetch" ) +type recordingProgress struct { + starts []fetch.ProgressInfo + adds []int64 + dones []error +} + +func (p *recordingProgress) Start(info fetch.ProgressInfo) fetch.ProgressTracker { + p.starts = append(p.starts, info) + return (*recordingTracker)(p) +} + +type recordingTracker recordingProgress + +func (t *recordingTracker) Add(n int64) { + p := (*recordingProgress)(t) + p.adds = append(p.adds, n) +} + +func (t *recordingTracker) Done(err error) { + p := (*recordingProgress)(t) + p.dones = append(p.dones, err) +} + func TestDownload(t *testing.T) { t.Run("downloads valid yaml", func(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -84,4 +108,101 @@ func TestDownload(t *testing.T) { t.Fatalf("unexpected error: %v", err) } }) + + t.Run("reports progress for known size", func(t *testing.T) { + body := []byte("shell: bash") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Accept-Encoding"); got == "gzip" { + t.Fatalf("fetch should not request gzip because it hides content length") + } + + w.Header().Set("Content-Type", "application/yaml") + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + _, _ = w.Write(body) + })) + defer srv.Close() + + progress := &recordingProgress{} + _, err := fetch.Download( + t.Context(), + srv.URL, + fetch.WithProgress(fetch.SourceRemoteConfig, progress), + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(progress.starts) != 1 { + t.Fatalf("expected one progress start, got %d", len(progress.starts)) + } + if progress.starts[0].Kind != fetch.SourceRemoteConfig { + t.Fatalf("unexpected progress kind: %q", progress.starts[0].Kind) + } + if progress.starts[0].URL != srv.URL { + t.Fatalf("unexpected progress url: %q", progress.starts[0].URL) + } + if progress.starts[0].TotalBytes != int64(len(body)) { + t.Fatalf("expected total %d, got %d", len(body), progress.starts[0].TotalBytes) + } + if sum(progress.adds) != int64(len(body)) { + t.Fatalf("expected add total %d, got %d", len(body), sum(progress.adds)) + } + if len(progress.dones) != 1 || progress.dones[0] != nil { + t.Fatalf("expected successful done, got %#v", progress.dones) + } + }) + + t.Run("reports unknown size", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + w.(http.Flusher).Flush() + _, _ = w.Write([]byte("shell: bash")) + })) + defer srv.Close() + + progress := &recordingProgress{} + _, err := fetch.Download( + t.Context(), + srv.URL, + fetch.WithProgress(fetch.SourceRemoteMixin, progress), + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(progress.starts) != 1 { + t.Fatalf("expected one progress start, got %d", len(progress.starts)) + } + if progress.starts[0].TotalBytes > 0 { + t.Fatalf("expected unknown total, got %d", progress.starts[0].TotalBytes) + } + }) + + t.Run("does not start progress before validation", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("{}")) + })) + defer srv.Close() + + progress := &recordingProgress{} + _, err := fetch.Download( + t.Context(), + srv.URL, + fetch.WithProgress(fetch.SourceRemoteConfig, progress), + ) + if err == nil { + t.Fatal("expected error") + } + if len(progress.starts) != 0 { + t.Fatalf("expected no progress starts, got %d", len(progress.starts)) + } + }) +} + +func sum(values []int64) int64 { + var total int64 + for _, value := range values { + total += value + } + + return total } diff --git a/internal/logging/formatter.go b/internal/logging/formatter.go index 0b4a8322..20508b7d 100644 --- a/internal/logging/formatter.go +++ b/internal/logging/formatter.go @@ -51,6 +51,7 @@ func newFormatter(errWriter io.Writer, cs fang.ColorSchemeFunc) *Formatter { if err != nil || w == 0 { w = 160 } + if w > 160 { w = 160 } @@ -99,6 +100,7 @@ func (f *Formatter) formatStyledError(entry *log.Entry) []byte { buf.WriteString("\n") buf.WriteString(f.errorStyles.text.Render(capitalizeFirst(entry.Message) + ".")) buf.WriteString("\n\n") + return buf.Bytes() } @@ -137,7 +139,9 @@ func capitalizeFirst(s string) string { if s == "" { return s } + runes := []rune(s) runes[0] = unicode.ToUpper(runes[0]) + return string(runes) } diff --git a/internal/theme/theme.go b/internal/theme/theme.go index b4a04267..4f6b9a68 100644 --- a/internal/theme/theme.go +++ b/internal/theme/theme.go @@ -2,9 +2,11 @@ package theme import ( "image/color" + "os" "charm.land/lipgloss/v2" "github.com/charmbracelet/x/exp/charmtone" + "github.com/charmbracelet/x/term" "github.com/lets-cli/fang" ) @@ -37,6 +39,37 @@ func ColorSchemeByName(name string) fang.ColorSchemeFunc { } } +// ProgressColorsByName resolves a theme name to fill and empty colors for download progress. +func ProgressColorsByName(name string, out term.File) (color.Color, color.Color) { + isDark := lipgloss.HasDarkBackground(os.Stdin, out) + scheme := ColorSchemeByName(name)(lipgloss.LightDark(isDark)) + + return ProgressColors(scheme) +} + +// ProgressColors resolves a Fang color scheme to fill and empty colors for download progress. +func ProgressColors(scheme fang.ColorScheme) (color.Color, color.Color) { + fill := scheme.Command + if fill == nil { + fill = scheme.Flag + } + + if fill == nil { + fill = scheme.Base + } + + empty := scheme.Comment + if empty == nil { + empty = scheme.FlagDefault + } + + if empty == nil { + empty = scheme.Base + } + + return fill, empty +} + // DefaultColorScheme is the default colorscheme. func DefaultColorScheme(c lipgloss.LightDarkFunc) fang.ColorScheme { baseCyan := charmtone.Turtle diff --git a/internal/theme/theme_test.go b/internal/theme/theme_test.go index 37f54cac..69356260 100644 --- a/internal/theme/theme_test.go +++ b/internal/theme/theme_test.go @@ -36,3 +36,21 @@ func TestColorSchemeByName(t *testing.T) { t.Fatalf("expected unknown theme to fall back to %v, got %v", charmtone.Ash, got) } } + +func TestProgressColors(t *testing.T) { + fill, empty := ProgressColors(ColorSchemeByName(DefaultName)(lipgloss.LightDark(true))) + if fill != charmtone.Turtle { + t.Fatalf("expected default fill color %v, got %v", charmtone.Turtle, fill) + } + if empty != lipgloss.Color("#747282") { + t.Fatalf("expected default empty color %v, got %v", lipgloss.Color("#747282"), empty) + } + + fill, empty = ProgressColors(ColorSchemeByName(ANSIName)(lipgloss.LightDark(true))) + if fill != lipgloss.White { + t.Fatalf("expected ansi fill color %v, got %v", lipgloss.White, fill) + } + if empty != lipgloss.BrightBlack { + t.Fatalf("expected ansi empty color %v, got %v", lipgloss.BrightBlack, empty) + } +}