gorilla / schema

Package gorilla/schema fills a struct with form values.
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
1.39k stars 231 forks source link

[FEATURE] Use omitempty by default #216

Open david-its opened 3 months ago

david-its commented 3 months ago

Is there an existing feature request for this?

Is your feature request related to a problem? Please describe.

We heavily use schema to encode request parameters in URL and to avoid adding "omitempty" in all tags would like to have an ability to make it default behavior by calling Encoder.SetDefaultOmitEmpty(true). If no objections we can submit a patch for that.

Describe the solution that you would like.

A new method Encoder.SetDefaultOmitEmpty(true) which makes omitempty to be a default behavior.

Describe alternatives you have considered.

No response

Anything else?

No response

zak905 commented 2 months ago

Hi @david-its, I was able to reproduce the issue, off course this would happen only when encoding. The following can be used as workaround:

    var probe *bool
    encoder.RegisterEncoder(probe, func(v reflect.Value) string {
        if v.IsNil() {
            return "false"
        }

        return fmt.Sprintf("%v", v.Elem().Bool())
    })

are you willing to provide an implementation to the solution ?

david-its commented 2 months ago

Such workaround is not correct unfortunately. When we use *bool we support tristate options (undefined, true, false). With this workaround you get only 2 states (true/false) which is not the same.

I'm not sure which solution is better:

2nd solution is very simple and I can create a PR.

zak905 commented 2 months ago

awesome thanks. I am also leaning towards option 2, because using a flag, we can keep backward compatibility, just in case someone is misusing the old behavior.

Yes, indeed, I overlooked the case where the value is nil, in this case an empty string should be returned. a key will be present in the values without a value, but this is ok I guess when dealing with form submissions or query parameters

var probe *bool
encoder.RegisterEncoder(probe, func(v reflect.Value) string {
        if v.IsNil() {
            return ""
        }

        return fmt.Sprintf("%v", v.Elem().Bool())
    })
david-its commented 2 months ago

awesome thanks. I am also leaning towards option 2, because using a flag, we can keep backward compatibility, just in case someone is misusing the old behavior.

Yes, indeed, I overlooked the case where the value is nil, in this case an empty string should be returned. a key will be present in the values without a value, but this is ok I guess when dealing with form submissions or query parameters

var probe *bool
encoder.RegisterEncoder(probe, func(v reflect.Value) string {
      if v.IsNil() {
          return ""
      }

      return fmt.Sprintf("%v", v.Elem().Bool())
  })

@zak905 this doesn't work as well, cause decoder automatically allocates zero values to fields even if there is no value provided. If I remember correctly, it happens at lines 306-312 in decode():

        if t.Kind() == reflect.Ptr {
                t = t.Elem()
                if v.IsNil() {
                        v.Set(reflect.New(t))
                }
                v = v.Elem()
        }

as a result you never have nil ptr in the field even though you decode field=.

I have submitted PR for default omit empty.