add .LicensePath and licenseText func to templates #179
Conversation
* Add `.LicensePath` to report templates reflecting the local file path of the license * Add a `licenseText` function to templates that takes a lib as an argument and renders the license text, if available Add a small template example to the readme
report.go
Outdated
| funcMap := template.FuncMap{ | ||
| "licenseText": func(lib libraryData) (string, error) { | ||
| if lib.LicensePath == "" { | ||
| return "", nil |
There was a problem hiding this comment.
nit: return some error message here for debugging?
like error: license path is empty?
There was a problem hiding this comment.
could do; you'd have to guard your use of it then by testing .LicensePath in your templates, but that's probably a better approach.
Also occurred to me that instead of adding a function to the template, i could add a method to libraryData and then template would then use {{ call .LicenseText }} .. not sure if that's better vs just different.
There was a problem hiding this comment.
could do; you'd have to guard your use of it then by testing .LicensePath in your templates, but that's probably a better approach.
Not sure I follow. I meant returning string error message that will be rendered by the template. Are our understanding the same?
Re method vs function, yes, method is better. People should not pass other objects.
There was a problem hiding this comment.
i assumed you meant return "", errors.New("no license found") which would cause the entire template not to render..
I don't think I'd want to return an error string in place of the license text (e.g. return "no license found", nil)
There was a problem hiding this comment.
return an error string in place of the license text
That's exactly what I meant.
On second thought, I agree it is not very helpful. When you render entire license text, the file is too large to read. Most likely no one can find an error message in the middle.
Shall we log an error message here instead? This should not be silently ignored.
There was a problem hiding this comment.
i don't think it should log an error if it's documented behaviour - If LicensePath is empty, it returns an empty string. An error would be failing to read from that path.
I personally just wrap it with a check:
{{ if .LicensePath }}
```
{{ .LicenseText }}
```
{{ end }}
I pushed an update that switches to using a method instead of adding a template function in any case
There was a problem hiding this comment.
Thank you! Adding clear doc on corner cases help remove unexpected behaviors.
That also reminds me .LicenseName is already a good enough indicator for license not found. So there is no need to check this case for this method.
In the future, if there could be expected cases that LicenseName is non empty but LicensePath is... we might need to revisit the decision here.
Add a LicenceText method
gwatts
force-pushed
the
add-licensetext-to-templates
branch
from
e27cf16 to
f986c4e
Compare
|
Awesome! Thank you for the contribution! |
|
@Bobgy thanks for merging! Appreciate all your work on this project; super useful |
Maybe resolves #144
.LicensePathto report templates reflecting the local file path of the licenselicenseTextfunction to templates that takes a lib as an argument and renders the license text, if availableAdd a small template example to the readme