chore: Refactoring attr doc gen to use Go templates #274
Conversation
traut
requested review from
Andrew-Morozko and
dobarx
and removed request for
Andrew-Morozko
| @@ -35,9 +36,9 @@ func makeHTTPDataSource(version string) *plugin.DataSource { | |||
| Header: dataspec.HeadersSpec{ | |||
| dataspec.ExactMatcher{"basic_auth"}, | |||
| }, | |||
| Doc: ` | |||
| Doc: u.Dedent(` | |||
There was a problem hiding this comment.
I don't think it's good idea to require correctness of Doc formatting at each plugin Spec.
"Dedent" it at doc generator or where you want to print this Doc string.
There was a problem hiding this comment.
I've rolled it back, looked at the code, thought about it more, and re-applied the change.
It makes sense to have clean data from the start: it removes the expectation that some code down the stream will clean the value up and the uncertainty about what exactly the clean-up does. There are 2 separate places where this clean-up happens at the moment. If we add another way to use the Doc value, we'll need to copy-paste it into the 3rd place, or abstract the code away into the attr schema struct. Or we can avoid all of that by passing a clean string in the first place with minimum complexity.
Moreover, we avoid uncertainty on how the text would be mutated during formatting, giving control to the plugin author, instead of doing it our job to clean messy doc string (and it might be messy in all different ways) before formatting.
There was a problem hiding this comment.
going to roll it in for now and revisit later!
| @@ -68,71 +67,47 @@ func MemoizedKeys[M ~map[string]V, V any](m *M) func() string { | |||
| }) | |||
| } | |||
|
|
|||
| const defaultTabSize = 4 | |||
| // Strip common margin from the beginnings of the lines | |||
| func Dedent(text string) string { | |||
There was a problem hiding this comment.
I would avoid adding new functions to util package. In general packages like util common helpers are anti-pattern in Go. You should add this logic where you really need it (docgen maybe?).
There was a problem hiding this comment.
Noted! In this case, dedent should really be part of the stdlib :D and we're using it in 2 separate places already, so I suspect it will stay in utils
No description provided.