diff icon indicating copy to clipboard operation
diff copied to clipboard

option for ignoring unexported fields

Open aexvir opened this issue 2 years ago • 3 comments

👋🏻 hey there I'm using this library as it has been the most consistent way to compare structs that I've found so far, thanks!

one feature that I'm missing is the option to ignore unexported fields from being compared and marked as differences, I know I can strip them by marshal/unmarshal but if that functionality would be provided by the library itself that would be amazing 🙂

I'm ok with implementing it myself and submitting a pr! I would like to know

  • is this something you're ok with adding?
  • is there some limitation that I could hit while adding this feature?
  • any hints where I should start from?

I haven't checked the codebase yet, that's why I'm asking this 😅

aexvir avatar Jul 13 '22 13:07 aexvir

well... I just realized I can use the Filter function 😅 🙈

diff.Filter(
    func(path []string, parent reflect.Type, field reflect.StructField) bool {
        return field.IsExported()
    },
)

hmm... I guess that's the way to handle it, right? I'd be ok with either documenting it as example or adding an explicit option for this imo that would make it more obvious

wdyt?

aexvir avatar Jul 13 '22 13:07 aexvir

Hey @aexvir , thanks for raising the issue!

So there are a couple of ways to handle this:

  1. use diff.Filter as you suggest
  2. use diff:"-" tag on your structs

So 2 would be the best way to go if you have control over how those structs are defined:

type MyStruct struct {
    DontDiff string `diff:"-"`  // this will be ignored
    Diff     string             // will be diffed
}

I hope that helps!

purehyperbole avatar Jul 13 '22 17:07 purehyperbole

while I was aware of the diff:"-" struct tag I don't want to keep track of this and make sure to add it every time we add an unexported field; the filter works alright though, so I'm happy with that

I still think that maybe documenting this would be nice, as it didn't struck me as something obvious on the first place but up to you 🙂 feel free to close this issue if there is nothing else to discuss here

and thanks for the help!

aexvir avatar Jul 15 '22 07:07 aexvir