Skip to content

feat: add insertion point for typedef#296

Merged
HeyJavaBean merged 2 commits intocloudwego:mainfrom
wolfogre:feature/typedef_insertion_point
Sep 24, 2025
Merged

feat: add insertion point for typedef#296
HeyJavaBean merged 2 commits intocloudwego:mainfrom
wolfogre:feature/typedef_insertion_point

Conversation

@wolfogre
Copy link
Contributor

@wolfogre wolfogre commented Sep 23, 2025

Description

Add an insertion point for typedef to provide convenience for plugins that add some stuff for typedef.

Motivation and Context

I built a plugin to support string enums, it works like

typedef string Person (
  datatype = "enum",
  datatype.enum.john = "john",
  datatype.enum.mary = "mary",
  datatype.enum.jack = "jack",
)
thriftgo -g go:use_type_alias=false,no_default_serdes -plugin datatype <path-to-thrift-file>
type Person string

const (
	PersonJohn Person = "john"
	PersonMary Person = "mary"
	PersonJack Person = "jack"
)

func (v Person) IsValid() error {
	switch v {
	case PersonJohn:
		return nil
	case PersonMary:
		return nil
	case PersonJack:
		return nil
	default:
		return fmt.Errorf("invalid Person: %q", v)
	}
}

func PersonList() []Person {
	return []Person{
		PersonJohn,
		PersonMary,
		PersonJack,
	}
}

It works well; however, there's no InsertionPoint in the template for typedef, so I have to use the eof point, which makes the generated code hard to read: the additional code is far away from the definition of the type. 😢

Related Issue

None.

@wolfogre wolfogre requested review from a team as code owners September 23, 2025 03:34
@HeyJavaBean
Copy link
Member

Thanks for your contribution. I understand your case, but I don't think it's necessary😢. Adding a new insertion point doesn't solve bugs or bring new features, just for code format, so I think the complexity it introduces outweighs the benefits.

@wolfogre
Copy link
Contributor Author

wolfogre commented Sep 23, 2025

@HeyJavaBean Thank you for your explanation.

However, I earnestly request a reconsideration of this.

Many other templates have one or more insertion points of their own.

{{InsertionPoint "constant" .Name}}

{{InsertionPoint "enum" .Name}}

{{InsertionPoint "service" .Name}}

All of them are valuable, right?

So does typedef also deserve to have one just like them?

@HeyJavaBean
Copy link
Member

ok

@HeyJavaBean HeyJavaBean merged commit 3464bbb into cloudwego:main Sep 24, 2025
4 checks passed
@wolfogre
Copy link
Contributor Author

@HeyJavaBean Thanks for reconsidering and merging this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants