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

all: refactor to use %q instead of "%s" #2001

Closed
wants to merge 1 commit into from
Closed

all: refactor to use %q instead of "%s" #2001

wants to merge 1 commit into from

Conversation

callthingsoff
Copy link
Contributor

This PR simplifies code using %q rather than "%s".

Please take a look. Thanks.

@github-actions github-actions bot added the area/docs-generation Generation of docs via Cobra label Jul 20, 2023
Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Looks safe. Thanks!

Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

CI disagrees that it’s safe. I didn’t look at what the error was though.

@callthingsoff
Copy link
Contributor Author

Thanks. I think it's this code.

fmt.Errorf(`required flag(s) "%s" not set`, strings.Join(missingFlagNames, `", "`))

It is now reverted. Thanks again.

@callthingsoff
Copy link
Contributor Author

Please take a look at this.

package main

import (
	"fmt"
	"path"
	"path/filepath"
	"strings"
	"time"
)

func main() {
	const fmTemplate = `---
date: %s
title: %q
slug: %s
url: %s
---
`
	filePrepender := func(filename string) string {
		now := time.Now().Format(time.RFC3339)
		name := filepath.Base(filename)
		base := strings.TrimSuffix(name, path.Ext(name))
		url := "/commands/" + strings.ToLower(base) + "/"
		return fmt.Sprintf(fmTemplate, now, strings.Replace(base, "_", " ", -1), base, url)
	}
	fmt.Printf("[%v]\n", filePrepender(`"abc".txt`))
}

The output is

[---
date: 2023-07-21T13:37:27+08:00
title: "\"abc\""
slug: "abc"
url: /commands/"abc"/
---
]

And the original output of "%s" is

[---
date: 2023-07-21T13:38:14+08:00
title: ""abc""
slug: "abc"
url: /commands/"abc"/
---
]

I don't know which style is better. Maybe this PR is unnecessary and I'll close it.

@marckhouzam
Copy link
Collaborator

I don't know which style is better. Maybe this PR is unnecessary and I'll close it.

yeah, with the extra escaping being done I see that %q is not exactly “%s”. Although it may be better in some cases let’s avoid changing anything that may affect existing projects.

Thanks anyway.

@callthingsoff callthingsoff deleted the fmt_sprintf_q branch July 21, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs-generation Generation of docs via Cobra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants