Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ title: Changelog
* `[Added]` Support `env_file` in global config and commands. File names are expanded after `env` is resolved, and values loaded from env files override values from `env`.
* `[Changed]` Migrate the LSP YAML parser from the CGO-based tree-sitter bindings to pure-Go [`gotreesitter`](https://github.com/odvcencio/gotreesitter), removing the C toolchain requirement from normal builds and release packaging.
* `[Refactoring]` Move CLI startup flow from `cmd/lets/main.go` into `internal/cli/cli.go`, keeping `main.go` as a thin launcher.
* `[Added]` Show background update notifications for interactive sessions, with Homebrew-aware guidance and `LETS_NO_UPDATE_NOTIFIER` opt-out.
* `[Added]` Add `lets self doc` command to open the online documentation in a browser.

## [0.0.59](https://github.com/lets-cli/lets/releases/tag/v0.0.59)
Expand Down
1 change: 1 addition & 0 deletions docs/docs/env.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ title: Environment
* `LETS_DEBUG` - enable debug messages
* `LETS_CONFIG` - changes default `lets.yaml` file path (e.g. LETS_CONFIG=lets.my.yaml)
* `LETS_CONFIG_DIR` - changes path to dir where `lets.yaml` file placed
* `LETS_NO_UPDATE_NOTIFIER` - disables background update checks and notifications
* `NO_COLOR` - disables colored output. See https://no-color.org/

### Environment variables available at command runtime
Expand Down
89 changes: 89 additions & 0 deletions internal/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package cli
import (
"context"
"errors"
"fmt"
"os"
"os/signal"
"strings"
"syscall"
"time"

"github.com/lets-cli/lets/internal/cmd"
"github.com/lets-cli/lets/internal/config"
Expand All @@ -17,10 +19,18 @@ import (
"github.com/lets-cli/lets/internal/upgrade"
"github.com/lets-cli/lets/internal/upgrade/registry"
"github.com/lets-cli/lets/internal/workdir"
"github.com/mattn/go-isatty"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)

const updateCheckTimeout = 3 * time.Second

type updateCheckResult struct {
notifier *upgrade.UpdateNotifier
notice *upgrade.UpdateNotice
}

func Main(version string, buildDate string) int {
ctx := getContext()

Expand Down Expand Up @@ -118,6 +128,9 @@ func Main(version string, buildDate string) int {
return 0
}

updateCh, cancelUpdateCheck := maybeStartUpdateCheck(ctx, version, command)
defer cancelUpdateCheck()

if err := rootCmd.ExecuteContext(ctx); err != nil {
var depErr *executor.DependencyError
if errors.As(err, &depErr) {
Expand All @@ -128,6 +141,8 @@ func Main(version string, buildDate string) int {
return getExitCode(err, 1)
}

printUpdateNotice(updateCh)

return 0
}

Expand Down Expand Up @@ -186,6 +201,80 @@ func allowsMissingConfig(current *cobra.Command) bool {
return false
}

func maybeStartUpdateCheck(
ctx context.Context,
version string,
command *cobra.Command,
) (<-chan updateCheckResult, context.CancelFunc) {
if !shouldCheckForUpdate(command.Name(), isInteractiveStderr()) {
return nil, func() {}
}

notifier, err := upgrade.NewUpdateNotifier(registry.NewGithubRegistry(ctx))
if err != nil {
log.Debugf("lets: update notifier init failed: %s", err)
return nil, func() {}
}

ch := make(chan updateCheckResult, 1)
checkCtx, cancel := context.WithTimeout(ctx, updateCheckTimeout)

go func() {
notice, err := notifier.Check(checkCtx, version)
if err != nil {
upgrade.LogUpdateCheckError(err)
}

ch <- updateCheckResult{
notifier: notifier,
notice: notice,
}
}()

return ch, cancel
}

func printUpdateNotice(updateCh <-chan updateCheckResult) {
if updateCh == nil {
return
}

select {
case result := <-updateCh:
if result.notice == nil {
return
}
Comment on lines +237 to +246
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Non-blocking receive can easily skip update notices and leaves the goroutine potentially blocked on send.

Because of the select { case <-updateCh: ... default: }, if printUpdateNotice runs before the goroutine sends to updateCh, the default branch is taken and the notice is never printed, even if the check succeeds shortly after. If the send happens after printUpdateNotice returns, the goroutine will block forever on ch <- updateCheckResult. Consider either using a blocking receive here (the check already has a timeout) or making the goroutine’s send non-blocking and treating the update as best-effort so neither side can block indefinitely and the notice is printed once the check completes.


if _, err := fmt.Fprintln(os.Stderr, result.notice.Message()); err != nil {
log.Debugf("lets: update notifier print failed: %s", err)
return
}

if err := result.notifier.MarkNotified(result.notice); err != nil {
upgrade.LogUpdateCheckError(err)
}
default:
}
}

func shouldCheckForUpdate(commandName string, interactive bool) bool {
if !interactive || os.Getenv("CI") != "" || os.Getenv("LETS_NO_UPDATE_NOTIFIER") != "" {
return false
}

switch commandName {
case "completion", "help", "lsp", "self":
return false
default:
return true
}
}

func isInteractiveStderr() bool {
fd := os.Stderr.Fd()
return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd)
}

type flags struct {
config string
debug int
Expand Down
34 changes: 34 additions & 0 deletions internal/cli/cli_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,40 @@
package cli

import "testing"

func TestShouldCheckForUpdate(t *testing.T) {
t.Run("should allow normal interactive commands", func(t *testing.T) {
if !shouldCheckForUpdate("lets", true) {
t.Fatal("expected update check to be enabled")
}
})

t.Run("should skip non interactive sessions", func(t *testing.T) {
if shouldCheckForUpdate("lets", false) {
t.Fatal("expected non-interactive session to skip update check")
}
})

t.Run("should skip when CI is set", func(t *testing.T) {
t.Setenv("CI", "1")
if shouldCheckForUpdate("lets", true) {
t.Fatal("expected CI to skip update check")
}
})

t.Run("should skip when notifier disabled", func(t *testing.T) {
t.Setenv("LETS_NO_UPDATE_NOTIFIER", "1")
if shouldCheckForUpdate("lets", true) {
t.Fatal("expected opt-out env to skip update check")
}
})

t.Run("should skip internal commands", func(t *testing.T) {
for _, name := range []string{"completion", "help", "lsp", "self"} {
if shouldCheckForUpdate(name, true) {
t.Fatalf("expected %q to skip update check", name)
}
import (

Check failure on line 37 in internal/cli/cli_test.go

View workflow job for this annotation

GitHub Actions / test-unit (ubuntu-latest)

expected statement, found 'import'
"testing"

cmdpkg "github.com/lets-cli/lets/internal/cmd"
Expand Down
Loading
Loading