introduce inline templates for Macros #198
Conversation
|
Noticed a bug |
|
Not a bug, but me not understanding the levels of parsing. |
| err, | ||
| "unable to unmarshal macros config template", | ||
| ) | ||
| return nil |
There was a problem hiding this comment.
should not we return the error here and below? karma.Format only adds a context-like message to an error.
There was a problem hiding this comment.
I copied the error handing from the error handling already present there. You must know: I'm just a go-beginner and mostly learn by looking at other's code. So I assumed the error handling here
Lines 138 to 142 in 4c81274
| ) | ||
| return nil | ||
| } | ||
| body, _ := cfg[template[1:]].(string) |
There was a problem hiding this comment.
Will it always (like in 100% cases) go well? It doesn't feel too safe actually. Let's check that there is such a field in cfg first with comma-ok pattern?
There was a problem hiding this comment.
As far as I understand, yes.
template is a string of at least 1 character, which will be '#' due to https://github.com/Skeeve/mark/blob/b7a30d6cf4e3fe0984cea44b4101e512b247fdfb/pkg/mark/macro/macro.go#L137
Also body will either be the inline macro's content or an empty string as, as far as I know, in case of a not-present map element, the result will be the empty string here:
So while it might produce unexpected results, it shouldn't crash.
But yes… An appropriate error message might be more helpful.
Templates in macros have to be either predefined ones (
ac:…) or be defined in additional files.With this PR one can use inline templates.