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 new notes output to get command #153

Merged
merged 1 commit into from
Jun 11, 2024
Merged
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
44 changes: 43 additions & 1 deletion cmd/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,32 @@ import (
"github.com/spf13/cobra"
)

type outputEnum string

const (
outputStandard outputEnum = "standard"
outputNotes outputEnum = "notes"
)

func (e *outputEnum) String() string {
return string(*e)
}

func (e *outputEnum) Set(v string) error {
switch v {
case string(outputStandard), string(outputNotes):
*e = outputEnum(v)
return nil
default:
return fmt.Errorf(`must be one of %s or %s`, outputStandard, outputNotes)
}
}

func (e *outputEnum) Type() string {
return "outputEnum"
}
Comment on lines +16 to +39
Copy link
Owner

Choose a reason for hiding this comment

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

How about we keep the implementation inside the writer package?

I'll explain what I mean over the next few comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only for handling the command line arguments. Open to suggestions on how to remove some of the duplication.


var outputTemplate = outputStandard
Copy link
Owner

Choose a reason for hiding this comment

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

outputTemplate could become outputType which is just a string.

Suggested change
var outputTemplate = outputStandard
var outputType string

var printLatest bool
var printVersion string

Expand All @@ -36,23 +62,33 @@ This command is useful for creating and updating Release notes in GitHub.
RunE: func(command *cobra.Command, args []string) error {
fileName := configuration.Config.FileName

var tmplSrc string
Copy link
Owner

Choose a reason for hiding this comment

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

tmplSrc is no longer needed.

Suggested change
var tmplSrc string

var changelog changelog.Changelog
var err error

if printLatest {
changelog, err = get.GetLatest(fileName)
} else if printVersion != "" {
changelog, err = get.GetVersion(fileName, printVersion)
} else if outputTemplate == outputNotes {
Copy link
Owner

Choose a reason for hiding this comment

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

For this comparison, we can just do a simple string one:

Suggested change
} else if outputTemplate == outputNotes {
} else if outputType == "notes" {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

outputType makes sense, but comparing to a string does not. All of this ties back to command flag validation and using constants to expose issues early, at compile time.

err = fmt.Errorf("notes output only supported with latest or version")
} else {
changelog, err = get.GetAll(fileName)
}

switch outputTemplate {
case outputStandard:
tmplSrc = writer.TmplSrcStandard
case outputNotes:
tmplSrc = writer.TmplSrcNotes
}

Comment on lines +79 to +85
Copy link
Owner

Choose a reason for hiding this comment

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

We can remove l79 - l85

if err != nil {
return err
}

var buf bytes.Buffer
if err := writer.Write(&buf, changelog); err != nil {
if err := writer.Write(&buf, tmplSrc, changelog); err != nil {
Copy link
Owner

Choose a reason for hiding this comment

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

writer.Write should now expect a string

Suggested change
if err := writer.Write(&buf, tmplSrc, changelog); err != nil {
if err := writer.Write(&buf, outputType, changelog); err != nil {

return err
}

Expand All @@ -77,5 +113,11 @@ func init() {
"Prints a specific version from the changelog to stdout.",
)

getCmd.Flags().Var(
&outputTemplate,
"output",
fmt.Sprintf(`Output template. allowed: "%s" or "%s"`, outputStandard, outputNotes),
)
Comment on lines +116 to +120
Copy link
Owner

Choose a reason for hiding this comment

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

For the flag definition, we can use StringVar and set a default value of standard.

Suggested change
getCmd.Flags().Var(
&outputTemplate,
"output",
fmt.Sprintf(`Output template. allowed: "%s" or "%s"`, outputStandard, outputNotes),
)
getCmd.Flags().StringVar(
&outputType,
"output",
"standard",
fmt.Sprintf(`Output template. allowed: "%s" or "%s"`, "standard", "notes"),
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where does flag value validation happen?


getCmd.Flags().SortFlags = false
}
2 changes: 1 addition & 1 deletion cmd/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var newCmd = &cobra.Command{
return err
}

if err := writer.Write(f, changelog); err != nil {
if err := writer.Write(f, writer.TmplSrcStandard, changelog); err != nil {
Copy link
Owner

Choose a reason for hiding this comment

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

Simple change here - writer.Write just takes standard.

Suggested change
if err := writer.Write(f, writer.TmplSrcStandard, changelog); err != nil {
if err := writer.Write(f, "standard", changelog); err != nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't this trade compile time for run-time errors and then it needs more tests to prevent invalid values when it's implicit using types and constants?

return err
}

Expand Down
54 changes: 52 additions & 2 deletions internal/writer/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/chelnak/gh-changelog/pkg/changelog"
)

var tmplSrc = `<!-- markdownlint-disable MD024 -->
const tmplStandard = `<!-- markdownlint-disable MD024 -->
# Changelog

All notable changes to this project will be documented in this file.
Expand Down Expand Up @@ -79,7 +79,57 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
{{- end}}
`

func Write(writer io.Writer, changelog changelog.Changelog) error {
const tmplNotes = `{{range .GetEntries }}
{{- if .Security }}
### Security
{{range .Security}}
- {{.}}
{{- end}}
{{end}}
{{- if .Changed }}
### Changed
{{range .Changed}}
- {{.}}
{{- end}}
{{end}}
{{- if .Removed }}
### Removed
{{range .Removed}}
- {{.}}
{{- end}}
{{end}}
{{- if .Deprecated }}
### Deprecated
{{range .Deprecated}}
- {{.}}
{{- end}}
{{end}}
{{- if .Added }}
### Added
{{range .Added}}
- {{.}}
{{- end}}
{{end}}
{{- if .Fixed }}
### Fixed
{{range .Fixed}}
- {{.}}
{{- end}}
{{end}}
{{- if .Other }}
### Other
{{range .Other}}
- {{.}}
{{- end}}
{{end}}
{{- end}}`

const (
TmplSrcStandard = tmplStandard
TmplSrcNotes = tmplNotes
)
Comment on lines +127 to +130
Copy link
Owner

Choose a reason for hiding this comment

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

We would no longer need to expose these two consts


func Write(writer io.Writer, tmplSrc string, changelog changelog.Changelog) error {
Copy link
Owner

Choose a reason for hiding this comment

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

And Write would become:

func getTemplate(s string) string {
	switch s {
	case "standard":
		return tmplStandard
	case "notes":
		return tmplNotes
	default:
		return tmplStandard
	}
}

func Write(writer io.Writer, outputType string, changelog changelog.Changelog) error {
	tmplSrc := getTemplate(outputType)
	tmpl, err := template.New("changelog").Funcs(template.FuncMap{
		"getFirstCommit": func() string {
			git := gitclient.NewGitClient(exec.Command)
			commit, err := git.GetFirstCommit()
			if err != nil {
				return ""
			}
			return commit
		},
	}).Parse(tmplSrc)
	if err != nil {
		return err
	}

	return tmpl.Execute(writer, changelog)
}

tmpl, err := template.New("changelog").Funcs(template.FuncMap{
"getFirstCommit": func() string {
git := gitclient.NewGitClient(exec.Command)
Expand Down
10 changes: 9 additions & 1 deletion internal/writer/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func Test_ItWritesOutAChangelogInTheCorrectFormat(t *testing.T) {
mockChangelog.AddUnreleased([]string{"Unreleased 1", "Unreleased 2"})

var buf bytes.Buffer
err := writer.Write(&buf, mockChangelog)
err := writer.Write(&buf, writer.TmplSrcStandard, mockChangelog)

assert.NoError(t, err)

Expand All @@ -58,4 +58,12 @@ func Test_ItWritesOutAChangelogInTheCorrectFormat(t *testing.T) {
assert.Regexp(t, "### Other", buf.String())
assert.Regexp(t, "- Other 1", buf.String())
assert.Regexp(t, "- Other 2", buf.String())

buf.Reset()
err = writer.Write(&buf, writer.TmplSrcNotes, mockChangelog)

assert.NoError(t, err)

assert.NotRegexp(t, regexp.MustCompile(`## \[v1.0.0\]\(https:\/\/github.com\/repo-owner\/repo-name\/tree\/v1.0.0\)`), buf.String())
assert.NotRegexp(t, regexp.MustCompile(`\[Full Changelog\]\(https:\/\/github.com\/repo-owner\/repo-name\/compare\/v0.9.0\.\.\.v1.0.0\)`), buf.String())
}
Loading