refactor: initialize a Viper instance to replace with the global singleton

[shabbywu]

Should Viper be a global singleton or passed around?

The best practice is to initialize a Viper instance and pass that around when necessary.

Viper comes with a global instance (singleton) out of the box. Although it makes setting up configuration easy, using it is generally discouraged as it makes testing harder and can lead to unexpected behavior.

The global instance may be deprecated in the future. See spf13/viper#1855 for more details.

ref: https://github.com/spf13/viper?tab=readme-ov-file#should-viper-be-a-global-singleton-or-passed-around

[Copilot]

Pull request overview

This PR refactors the CLI configuration handling to use an explicitly-initialized *viper.Viper instance (rather than relying on Viper’s global singleton), improving testability and reducing reliance on global state.

Changes:

• Introduces a defaultViper instance initialized in initConfig() and passes it into command execution paths.
• Splits Cobra Run handlers into run*Cmd wrappers and run* helpers that accept a *viper.Viper.
• Renames several run* handlers for consistency (runClientCmd, runServerCmd, etc.).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
app/cmd/root.go	Creates defaultViper and wires it into config initialization.
app/cmd/client.go	Routes client execution through runClientCmd → runClient(*viper.Viper).
app/cmd/server.go	Routes server execution through runServerCmd → runServer(*viper.Viper).
app/cmd/ping.go	Updates ping command to use the injected Viper instance.
app/cmd/speedtest.go	Updates speedtest command to use the injected Viper instance.
app/cmd/share.go	Updates share command to use the injected Viper instance.
app/cmd/update.go	Adds Viper dependency and routes update check through a helper function.
app/cmd/version.go	Renames the version command handler for consistency.

Comments suppressed due to low confidence (1)

app/cmd/update.go:12

• The imports are not gofmt-compliant: standard library imports (e.g., time) should come before third-party imports, with a blank line separating groups. Running gofmt (or reordering the import block) will fix this and avoid CI/lint failures if formatting is enforced.

import (
	"github.com/spf13/viper"
	"time"

	"github.com/spf13/cobra"
	"go.uber.org/zap"

	"github.com/apernet/hysteria/app/v2/internal/utils"
	"github.com/apernet/hysteria/core/v2/client"
)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

runCheckUpdate accepts a *viper.Viper parameter but never uses it. This makes the function signature misleading and forces an otherwise unnecessary Viper dependency/import here. Consider removing the parameter (and passing no Viper from runCheckUpdateCmd), or actually using it if configuration is intended to influence update checks.

[Copilot]

The parameter name viper shadows the imported viper package inside this function body, which is easy to misread and prevents referring to the package name within the function. Consider renaming the parameter to something like vp, cfgViper, or cfg for clarity.

[Copilot]

The parameter name viper shadows the imported viper package inside this function body, which reduces readability. Consider renaming the parameter (e.g., vp/cfg) to avoid the shadowing.

[Copilot]

The parameter name viper shadows the imported viper package inside this function body, which can be confusing when scanning code and makes it impossible to refer to the package name within the function. Consider renaming the parameter (e.g., vp/cfg).

[Copilot]

The parameter name viper shadows the imported viper package inside this function body, which hurts readability and can make future edits awkward if package-level helpers are needed. Consider renaming the parameter (e.g., vp/cfgViper).

[Copilot]

The parameter name viper shadows the imported viper package inside this function body. Renaming it (e.g., vp/cfg) would make the code easier to read and avoid shadowing surprises.

[Copilot]

initConfig mutates the package-level viper.SupportedExts slice. That global mutation can accumulate duplicates if initConfig is invoked more than once (e.g., in tests) and partially undermines the goal of avoiding global Viper state. Consider avoiding this mutation (or guarding it with a sync.Once / dedup) and keeping all configuration on the defaultViper instance.