Skip to content

Support marshal with linebreak and indent and support ignore specific struct field#150

Open
funte wants to merge 7 commits intoinvopop:mainfrom
funte:main
Open

Support marshal with linebreak and indent and support ignore specific struct field#150
funte wants to merge 7 commits intoinvopop:mainfrom
funte:main

Conversation

@funte
Copy link
Copy Markdown

@funte funte commented Aug 30, 2024

  1. For human readable in debug, add some code to support Schema.MarshalJSON marshal json with linebreak and indent, the default json output is still compact;
var MarshalWithIndent = false
var MarshalPrefix = ""
var MarshalIndent = "\t"
  1. Add optional Reflector.Ignore method to ignore specific struct field, enable dynamic generate different schemas from one struct.
Ignore func(reflect.StructField) bool

Copy link
Copy Markdown
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

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

Tests are essential here to understand what you're trying to achieve. Also, I'm not sure indentation should be handled in the MarshalJSON method.

Comment thread reflect.go
Comment on lines +1092 to +1094
var MarshalWithIndent = false
var MarshalPrefix = ""
var MarshalIndent = "\t"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Global variables should be avoided. Wouldn't you want to do this with the regular json.MarshalIndent method directly?

Comment thread reflect.go
IgnoredTypes []any

// Ignore specific struct field, enable dynamic generate different schemas from on struct.
Ignore func(reflect.StructField) bool
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Its not clear how this should be used... as a rule, I'd try to avoid using reflect types directly.

@samlown samlown added the needs tests Not enough tests for this to be accepted label Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs tests Not enough tests for this to be accepted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants