Skip to content

add mapping and String() method for enum type in generated bindings#128

Merged
ydnar merged 1 commit intobytecodealliance:mainfrom
rajatjindal:enum-reverse-lookup
Jul 7, 2024
Merged

add mapping and String() method for enum type in generated bindings#128
ydnar merged 1 commit intobytecodealliance:mainfrom
rajatjindal:enum-reverse-lookup

Conversation

@rajatjindal
Copy link
Copy Markdown
Member

this adds String() method for enum type as discussed in #127

If this approach looks ok, I will also try to add for variant and flag types.

@rajatjindal
Copy link
Copy Markdown
Member Author

for variant lookup, it seems a bit odd:

e.g.

// NewTimestamp represents the variant "wasi:filesystem/types@0.2.0#new-timestamp".
//
// When setting a timestamp, this gives the value to set it to.
//
//	variant new-timestamp {
//		no-change,
//		now,
//		timestamp(datetime),
//	}

var mapNewTimestamp = map[uint8]string {
	0: "no-change",
	1: "now",
	2: "timestamp(datetime)",
}

func (self *NewTimestamp) String() string {
	return mapNewTimestamp[self.Tag()]
}

I am wondering if using String() method is a good choice for variant case as user might expect the string representation of actual value of NewTimestamp instead of just type of it.

Comment thread wit/bindgen/generator.go Outdated
Comment thread wit/bindgen/generator.go Outdated
Comment thread wit/bindgen/generator.go Outdated
Comment thread wit/bindgen/generator.go Outdated
@rajatjindal rajatjindal force-pushed the enum-reverse-lookup branch from 75608fd to fcedc99 Compare July 6, 2024 08:18
Copy link
Copy Markdown
Collaborator

@ydnar ydnar left a comment

Choose a reason for hiding this comment

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

See comments

@rajatjindal rajatjindal force-pushed the enum-reverse-lookup branch 3 times, most recently from 058aef5 to 714b997 Compare July 6, 2024 09:20
Copy link
Copy Markdown
Collaborator

@ydnar ydnar left a comment

Choose a reason for hiding this comment

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

Looking better! Just one small tweak.

I'd like to figure out how to add tests for this, but we can do that in a follow-up PR.

Comment thread wit/bindgen/generator.go Outdated
@rajatjindal rajatjindal force-pushed the enum-reverse-lookup branch from 714b997 to fb780ba Compare July 6, 2024 10:41
Copy link
Copy Markdown
Collaborator

@ydnar ydnar left a comment

Choose a reason for hiding this comment

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

Looks good. Some minor nits, but we can probably merge.

Comment thread wit/bindgen/generator.go Outdated
Comment thread wit/bindgen/generator.go Outdated
@rajatjindal rajatjindal force-pushed the enum-reverse-lookup branch from fb780ba to 3cbef77 Compare July 6, 2024 11:11
Comment thread wit/bindgen/generator.go
Signed-off-by: Rajat Jindal <rajatjindal83@gmail.com>
@rajatjindal rajatjindal force-pushed the enum-reverse-lookup branch from 3cbef77 to 780eb20 Compare July 6, 2024 11:28
@ydnar ydnar merged commit 16f9943 into bytecodealliance:main Jul 7, 2024
ydnar added a commit that referenced this pull request Jul 7, 2024
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