Skip to content
This repository has been archived by the owner on Dec 31, 2022. It is now read-only.

imgui.Textf for ease of use? #168

Closed
eliasdaler opened this issue Jan 6, 2022 · 1 comment · Fixed by #170
Closed

imgui.Textf for ease of use? #168

eliasdaler opened this issue Jan 6, 2022 · 1 comment · Fixed by #170

Comments

@eliasdaler
Copy link
Contributor

eliasdaler commented Jan 6, 2022

Hello.
I think it might be nice to add imgui.Textf which will work similarly to log.Printf and fmt.Errorf found in Go standard library - by doing fmt.Sprintf internally.

This will allow me to replace code like this:

imgui.Text(fmt.Sprintf("some text: %s", str))

with this:

imgui.Textf("some text: %s", str)

... and it will also be kinda similar to how C++'s ImGui::Text works (which allows you to do formatting by "default").
Any thoughts? :)

@dertseha
Copy link
Member

dertseha commented Jan 8, 2022

Hi @eliasdaler , and thank you for your message!

This one is a tough call.

This approach would be against the initial API philosophy about the wrapper - and the case for Text() is even listed explicitly as an example for that in the readme. Second, this would add a "new" function which is not available in the original library, and my intent was to keep this library as a thin wrapper as much as possible.

On the other hand, at least for the case of Text(), this decision was about avoiding potential problems with formatting strings and trying to forward any arbitrary references down to C-land; This would be mitigated by your proposal, doing something like

func Textf(format string, a ...interface{}) {
   Text(fmt.Sprintf(format, a...))
}

And second, it would be more idiomatic Go, like you wrote - and would be a quality of life feature.

With that reasoning I'm slightly tilting towards your proposal.
Please go ahead, provide a PR, which also rephrases the sample in readme.md - and see if there are any further cases where this applies (e.g. LabelText() & LabelTextf()).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants