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 support for emoji compilation in formatted arguments #32

Merged
merged 1 commit into from
Mar 3, 2019
Merged

add support for emoji compilation in formatted arguments #32

merged 1 commit into from
Mar 3, 2019

Conversation

bobheadxi
Copy link
Contributor

@bobheadxi bobheadxi commented Mar 1, 2019

I have some structs that implement String() string, where String() could return something that contains an emoji. However, when doing something like:

// type C implements String() string
Printf(":rocket: am I %s?\n", C("coloured :rocket:", RD, UL))
//  🚀  am I coloured :rocket:?

this happens because arguments to Printf or anything that takes ...interface{} are compiled separately argument-by-argument (example), and only compiles emojis if it is a string:

func compileValues(a *[]interface{}) {
	for i, x := range *a {
		if str, ok := x.(string); ok {
			(*a)[i] = compile(str)
		}
	}
}

there are 2 ways to support this:

  • in compileValues, check for String() via type assertion and compile that
  • call Sprint or the required variation of it before compiling, and make compile the last step before a print in every case

this PR implements the 2nd method, which I think is the cleanest and most consistent way to do it - if you prefer the other way, or have other ideas, I would be happy to implement it.

thanks!

@bobheadxi bobheadxi changed the title make compilation the last step in all printers add support for emoji compilation in formatted arguments Mar 1, 2019
Copy link
Owner

@kyokomi kyokomi left a comment

Choose a reason for hiding this comment

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

@bobheadxi Thank you for fixing. I think that this content is good. 👍

@kyokomi kyokomi merged commit 5176225 into kyokomi:master Mar 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants