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

proposal: hasField function for use with structs #60

Closed
1 task done
mbezhanov opened this issue Aug 22, 2024 · 1 comment · Fixed by #61
Closed
1 task done

proposal: hasField function for use with structs #60

mbezhanov opened this issue Aug 22, 2024 · 1 comment · Fixed by #61
Assignees
Labels
aspect/dex 🤖 Concerns developers' experience with the codebase domain/obvious 🟩 Represents the "known knowns" issue. It's Obviously good first issue Good for newcomers priority/low 🟩 Priority 4 - Low priority and doesn't need to be rushed state/todo 🚀 This is confirmed, will work on soon state/triage 🚦 Has not been triaged & therefore, not ready for work type/feature ⭐ Addition of new feature

Comments

@mbezhanov
Copy link

You have a proposal, explain it!

Originally proposed here: Masterminds/sprig#401

I recently worked on a project that used generics in a similar fashion:

type A[T any] struct {
	Metadata T
}

type B struct {
	Foo string
}
type C struct {
	Bar string
}

Then, similar objects were being passed to a template for rendering:

ab := &A[B]{Metadata: B{Foo: "Lorem"}}
ac := &A[C]{Metadata: C{Bar: "Ipsum"}}

A section in the template had to look slightly different depending on the type of metadata being passed in, which I solved by using a custom hasField method:

{{if hasField $.Metadata "Foo"}}
  We have Foo.
{{else}}
  We have no Foo.
{{end}}
Full code example
package main

import (
  "html/template"
  "log"
  "os"
  "reflect"
)

type A[T any] struct {
  Metadata T
}

type B struct {
  Foo string
}
type C struct {
  Bar string
}

const tmpl = `
  {{if hasField $.Metadata "Foo"}}
    We have Foo.
  {{else}}
    We have no Foo.
  {{end}}`

func main() {
  f := template.FuncMap{"hasField": hasField}
  t := template.Must(
  	template.New("base").Funcs(f).Parse(tmpl),
  )
  ab := &A[B]{Metadata: B{Foo: "Lorem"}}
  ac := &A[C]{Metadata: C{Bar: "Ipsum"}}
  if err := t.Execute(os.Stdout, ab); err != nil {
  	log.Fatal(err)
  }
  if err := t.Execute(os.Stdout, ac); err != nil {
  	log.Fatal(err)
  }
}

func hasField(v interface{}, name string) bool {
  rv := reflect.ValueOf(v)
  if rv.Kind() == reflect.Ptr {
  	rv = rv.Elem()
  }
  if rv.Kind() != reflect.Struct {
  	return false
  }
  return rv.FieldByName(name).IsValid()
}

Describe the solution you'd like

I took the hasField method implementation from the following StackOverflow thread: Field detection in Go HTML template.

It looks like this:

func hasField(v interface{}, name string) bool {
	rv := reflect.ValueOf(v)
	if rv.Kind() == reflect.Ptr {
		rv = rv.Elem()
	}
	if rv.Kind() != reflect.Struct {
		return false
	}
	return rv.FieldByName(name).IsValid()
}

I feel something similar can be added to the reflect registry, so it can be used with Sprout out of the box.

I'd be happy to open a PR!

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@mbezhanov mbezhanov added the state/triage 🚦 Has not been triaged & therefore, not ready for work label Aug 22, 2024
@42atomys 42atomys added good first issue Good for newcomers priority/low 🟩 Priority 4 - Low priority and doesn't need to be rushed state/todo 🚀 This is confirmed, will work on soon domain/obvious 🟩 Represents the "known knowns" issue. It's Obviously type/feature ⭐ Addition of new feature aspect/dex 🤖 Concerns developers' experience with the codebase labels Aug 22, 2024
@42atomys 42atomys assigned mbezhanov and unassigned 42atomys Aug 22, 2024
@42atomys
Copy link
Member

Hello @mbezhanov thanks for your proposal and your PR I will take a look soon

42atomys pushed a commit that referenced this issue Aug 22, 2024
## Description

The PR introduces a `hasField` method to the `reflect` registry, to
address the use case explained in #60.

## Fixes #60 

## Checklist
- [x] I have read the **CONTRIBUTING.md** document.
- [x] My code follows the code style of this project.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [x] I have updated the documentation accordingly.
- [x] This change requires a change to the documentation on the website.

---------

Signed-off-by: Atomys <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect/dex 🤖 Concerns developers' experience with the codebase domain/obvious 🟩 Represents the "known knowns" issue. It's Obviously good first issue Good for newcomers priority/low 🟩 Priority 4 - Low priority and doesn't need to be rushed state/todo 🚀 This is confirmed, will work on soon state/triage 🚦 Has not been triaged & therefore, not ready for work type/feature ⭐ Addition of new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants