Skip to content

Add support for exclusion of field via field tag.#51

Closed
NicklasWallgren wants to merge 2 commits intogkampitakis:mainfrom
NicklasWallgren:suppress-fields
Closed

Add support for exclusion of field via field tag.#51
NicklasWallgren wants to merge 2 commits intogkampitakis:mainfrom
NicklasWallgren:suppress-fields

Conversation

@NicklasWallgren
Copy link
Copy Markdown

@NicklasWallgren NicklasWallgren commented Oct 16, 2022

With these changes, one can add a struct field tag go-snaps:"-" to ignore pretty printing a particular field.

type testFieldTag struct {
	x, y int
	z string `go-snaps:"-"`
}

It's a good option until support for matchers has been released.

@NicklasWallgren NicklasWallgren changed the title Add support for exclude field tag. Add support for exclusion of field via field tag. Oct 16, 2022
@gkampitakis
Copy link
Copy Markdown
Owner

Hey, thanks a lot for the pr 😊 . Will have a look at it as soon as possible

@gkampitakis
Copy link
Copy Markdown
Owner

Sorry for the delay, first of all, thanks again for your contribution and for your interest in the library.

I have some concerns with this solution. First, it's pretty clever using the tags, I never thought about it, but I am worried people are already making the structs really "crowded" with tags, not sure they would want to add to that long list another one that would be for tests only.
Secondly, i am not sure I feel comfortable vendoring another module for functionality that matchJSON partially can already support. ( and possibly in the future creating a matchStruct() that can leverage the same matchers )

I am not saying no I am just sharing thoughts and asking for feedback 😅 potentially on the matchJSON functionality as well.

@gkampitakis gkampitakis added the enhancement New feature or request label Oct 20, 2022
@NicklasWallgren
Copy link
Copy Markdown
Author

NicklasWallgren commented Oct 21, 2022

Thanks for the reply!

I wasn't aware that you were working on adding support for property matchers when I created this PR. I noticed #18, and thought that I was forced to implement my own snapshot-library, with support for matchers.

I'm really glad you started working on matchJSON, since the current implementation doesn't take dynamic properties into account.

I'm not to comfortable about the vendoring either. There is actually an open PR on kr/pretty with similar support, kr/pretty#51.

@gkampitakis
Copy link
Copy Markdown
Owner

Hey 👋, just to keep you on the loop haven't forgot about this pr. Just waiting if there is some interest on merging the pr you shared on pretty

@gkampitakis gkampitakis added the wontfix This will not be worked on label Jan 21, 2024
@gkampitakis
Copy link
Copy Markdown
Owner

Closing this. If there is interest from pretty to imlpement it, I am happy to document this.

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

Labels

enhancement New feature or request wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants