-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix frontmatter issues #16
Conversation
Signed-off-by: Saswata Mukherjee <[email protected]>
7fd04ad
to
92d712c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this issue 🤗
Good work, some early suggestions 💪🏽
pkg/mdformatter/mdformatter.go
Outdated
for _, k := range keys { | ||
_, _ = fmt.Fprintf(b, "%v: %v\n", k, frontMatter[k]) | ||
// check if frontMatter[k] is a map | ||
frontMatterStr := fmt.Sprintf("%s", frontMatter[k]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice tricky, but wonder if we could add some type safety (: Would _, isMap := frontMatter[k].(map[string]interface{})
work? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will check it out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it worked! Do we need frontMatterStr
then? 🤗
Signed-off-by: Saswata Mukherjee <[email protected]>
6a56309
to
bb3e604
Compare
Linting issues have been fixed @bwplotka. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, almost perfect (:
pkg/mdformatter/mdformatter.go
Outdated
for _, k := range keys { | ||
_, _ = fmt.Fprintf(b, "%v: %v\n", k, frontMatter[k]) | ||
// check if frontMatter[k] is a map | ||
frontMatterStr := fmt.Sprintf("%s", frontMatter[k]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it worked! Do we need frontMatterStr
then? 🤗
pkg/mdformatter/mdformatter.go
Outdated
_, _ = fmt.Fprintf(b, "%v: %v\n", k, frontMatter[k]) | ||
// Check if frontMatter[k] is a map. | ||
frontMatterStr := fmt.Sprintf("%s", frontMatter[k]) | ||
_, isMap := frontMatter[k].(map[string]interface{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we can do better. Since you are casting to map we could use this map directly, no need to use frontMatterStr
, no? (:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes. Will make the changes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used the casted map. Code looks better now!
Signed-off-by: Saswata Mukherjee <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, thank you!
for _, k := range keys { | ||
_, _ = fmt.Fprintf(b, "%v: %v\n", k, frontMatter[k]) | ||
// Check if frontMatter[k] is a map. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the next time, this comment might be not needed - the code is now nicely self-explanatory (:
This PR fixes the frontmatter issue, where frontmatter in md being formatted led to
map
s showing up. Now,mdformatter.go
correctly processes nested fields. Fixes issue #13The above frontmatter generated before fix.
The above frontmatter generated after fix.