@olblak, PTAL

This PR fixes the immediate problem (banner corrupting completion scripts) by skipping the version check for generator commands and adding the --disable-version-check flag. But the root cause from #9028 is still open: logrus.SetOutput(os.Stdout) means the banner goes to stdout, so any command where someone pipes stdout could still get the banner mixed into their output

The issue suggested routing the banner to stderr as one of the fixes. I held off on that here to keep the scope small, but happy to include it in this PR if you'd prefer. Otherwise I can open a follow-up.

thanks!!

Thanks for the pullrequest, I like the environment variable and flag to totally disable the version checking as it can reduce http queries to updatecli.io

I am struggling to test this change manually or other changes.

I am curious what you think about my suggestion on #9028 (comment).
The idea would to only run the check version in a defer function so in theory it wouldn't be executed on auto completation.

Regarding moving the message to stderr, I don't know if it's the time to do it as I would like to revamp to UI and move away from logrus completely, but I never took the time

Thanks for the feedback @olblak!

Regarding the defer approach in run() - I think that's cleaner than the skip list. The skip list works but every new generator command needs to be manually added. With defer inside run(), the version check naturally only runs for commands that go through that function, so completion/docs/man/etc. are excluded without any extra logic.

The tradeoff is that some commands in run() like udash/login, udash/logout, manifest/pull probably don't need the version check either, but since they don't redirect stdout it's not harmful, just an extra network request. The skip list is more surgical, but defer is simpler and harder to get wrong.

I'd lean toward defer for simplicity, and keep the --disable-version-check flag either way so users can opt out regardless of which command they're running. That also helps with reducing HTTP queries to updatecli.io.

As for only checking on updatecli version - that feels a bit too restrictive since most users don't run that directly, so they'd never see the update notification. The current approach surfaces it during normal usage which is more useful.

Happy to refactor to the defer approach if that's your preference. And understood on stderr/logrus, makes sense to handle that as part of a larger UI revamp.

I could manually test it running

go build -ldflags="-X github.com/updatecli/updatecli/pkg/core/version.Version=v0.1.0" -o bin/updatecli .
./bin/updatecli completion bash > /tmp/completions.bash
grep -n 'new release' /tmp/completions.bash
bash -norc /tmp/completions.bash

Where we shouldn't see 427:| A new release is available: "0.1.0" -> "0.117.1"

After additional testing, your approach is better, as not every subcommand calls the run function
So, for example, my suggestion wouldn't work with updatecli version

Good catch on updatecli version @olblak, that would have been an ironic miss. Thanks for fixing the formatting too. Skip list it is, I'll keep an eye on CI.

looks like the analyse check hit a gitHub rate limit. mind re-running it when you get a chance @olblak?
thanks!

all checks passed @olblak. ready for merge whenever you are :)

Thank you for the pullrequest today I learn