Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to load config from file #92

Merged
merged 12 commits into from
May 6, 2023
Merged

Conversation

Kamsiy
Copy link
Contributor

@Kamsiy Kamsiy commented Apr 25, 2023

Description
This PR allows loading a custom style from a json or yaml file or an environment variable

Examples

Using a loaded style with a Printer

style, err := printer.WithPrettyStyleFile("version-style.yml")
p := printer.New(style)

// or 
// to load from an environment variable
style, err := printer.WithPrettyStyleFromEnv("version-style")
p := printer.New(style)

Using a loaded style with Cobra

style := printer.WithPrettyStyleFile("version-style.yml")
cmd := &cobra.Command{
	Use:   "example",
	Short: "An example CLI built with github.com/spf13/cobra",
}
cmd.AddCommand(
	extension.NewVersionCobraCmd(
		extension.WithPrinterOptions(&style),
	),
)

Related issue(s)

@Kamsiy Kamsiy marked this pull request as ready for review April 26, 2023 14:37
@mszostok mszostok self-requested a review April 27, 2023 07:07
@mszostok mszostok added enhancement New feature or request area/customization Enables default behavior customization labels Apr 27, 2023
Copy link
Owner

@mszostok mszostok left a comment

Choose a reason for hiding this comment

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

Thanks again for the PR, Awesome work! 🚀

The only reason that I didn't approve it is that it will cause a panic in such scenario:

func main() {
	// Add support for setting styles (optionally) via 'CLI_OUTPUT_STYLE' env
	styleFromEnv, err := printer.WithPrettyStyleFromEnv("CLI_OUTPUT_STYLE")
	if err != nil {
		log.Fatal()
	}

	p := printer.New(&styleFromEnv)
	if err := p.Print(os.Stdout); err != nil {
		log.Fatal(err)
	}
}

run:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xc0 pc=0x12ced80]

goroutine 1 [running]:
go.szostok.io/version/style.(*GoTemplateRender).Render(0xc0000122b8, {0x13e7c80, 0xc0001d40a0}, 0x98?)
        /Users/mszostok/workspace/go/src/github.com/mszostok/version/style/render.go:51 +0x940
go.szostok.io/version/printer.(*Pretty).execute(0x14e9a60?, 0xc000014018?, 0x83?)
        /Users/mszostok/workspace/go/src/github.com/mszostok/version/printer/pretty.go:89 +0x45
go.szostok.io/version/printer.(*Pretty).Print(0xc0000122d0, 0xc0001d40a0, {0x14e9a60, 0xc000014018})
        /Users/mszostok/workspace/go/src/github.com/mszostok/version/printer/pretty.go:71 +0x5f
go.szostok.io/version/printer.(*Container).PrintInfo(0xc00010fc80, {0x14e9a60, 0xc000014018}, 0xc0001d40a0)
        /Users/mszostok/workspace/go/src/github.com/mszostok/version/printer/printer.go:75 +0x70
go.szostok.io/version/printer.(*Container).Print(0xc000137f60?, {0x14e9a60, 0xc000014018})
        /Users/mszostok/workspace/go/src/github.com/mszostok/version/printer/printer.go:65 +0x3f
main.main()
        /Users/mszostok/workspace/go/src/github.com/mszostok/version/examples/custom-layout/main.go:18 +0xb2

When the env is specified, we cannot require that someone will always set it. It's up to the users to use it or not.

printer/printer_test.go Outdated Show resolved Hide resolved
printer/opts.go Outdated Show resolved Hide resolved
printer/opts.go Show resolved Hide resolved
printer/opts.go Outdated Show resolved Hide resolved
printer/printer_test.go Outdated Show resolved Hide resolved
@Kamsiy
Copy link
Contributor Author

Kamsiy commented May 6, 2023

Thanks, Good catch 😅. I've updated the PR to handle this

Copy link
Owner

@mszostok mszostok left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thanks a lot for working on this 🙂

@mszostok mszostok merged commit e9da3c1 into mszostok:main May 6, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/customization Enables default behavior customization enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants