Skip to content
This repository has been archived by the owner on Jun 6, 2022. It is now read-only.

Improve templating #96

Merged
merged 4 commits into from
Jan 24, 2019
Merged

Improve templating #96

merged 4 commits into from
Jan 24, 2019

Conversation

twaugh
Copy link
Collaborator

@twaugh twaugh commented Jan 18, 2019

The output template can now be more flexible, so that:

  • the full output line can be controlled, rather than only the text after the package name
  • the top-level package for a vendored package can be included in each output line

@twaugh twaugh force-pushed the templates branch 2 times, most recently from ed1b615 to 4c1f4ed Compare January 18, 2019 13:54
@twaugh twaugh changed the title [WIP] Improve templating Improve templating Jan 18, 2019
@twaugh twaugh requested a review from mprahl January 22, 2019 15:26
main.go Outdated
@@ -19,6 +19,7 @@ import (
"bufio"
"flag"
"fmt"
"html/template"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to make this html/template instead of text/template? html/template will escape HTML characters, which is likely undesired.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Nice catch. Fixed.

@mprahl
Copy link
Contributor

mprahl commented Jan 22, 2019

Have you considered using an option to write to a JSON file with all this information and then the caller can programmatically parse the JSON file rather than pass in a template and then parse the output text of retrodep?

Copy link
Contributor

@mprahl mprahl left a comment

Choose a reason for hiding this comment

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

👍 once the comments are considered

Previously the template string was being compiled every time it was
used. Instead, compile it only once at the start, then use the
compiled template each time it is needed.

Signed-off-by: Tim Waugh <[email protected]>
Signed-off-by: Tim Waugh <[email protected]>
@twaugh
Copy link
Collaborator Author

twaugh commented Jan 23, 2019

Have you considered using an option to write to a JSON file with all this information and then the caller can programmatically parse the JSON file rather than pass in a template and then parse the output text of retrodep?

That's a good idea. In fact we could have a -o option similar to ocs, allowing:

  • retrodep -o json ...
  • retrodep -o go-template="{{...}}" ...

This new -o go-template= option could be the way to specify a full-line template (eliminating the need for -new-template), with -template providing compatibility for the previous templating style.

@twaugh
Copy link
Collaborator Author

twaugh commented Jan 23, 2019

I've updated it to use -o go-template=..., to allow -o json in the future. I took a quick stab at working out how to make JSON output work but ran into questions when dealing with unknown versions. I filed #97 to track that separately.

@twaugh twaugh requested a review from vkrizan January 23, 2019 14:39
@vkrizan
Copy link
Collaborator

vkrizan commented Jan 23, 2019

Looks good. Great idea of having more than one output format with -o.

This commit adds a command line argument -o to control output format.
The only defined format is go-template=... which does not add the
package name by default, unlike the now-deprecated -template argument
which always starts with the package name.

Signed-off-by: Tim Waugh <[email protected]>
@twaugh
Copy link
Collaborator Author

twaugh commented Jan 23, 2019

Updated README.md.

README.md Outdated
-importpath string
top-level import path
top-level import path
-new-template
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please reflect argument changes to -o [go-template=...].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, updated.

Copy link
Contributor

@mprahl mprahl left a comment

Choose a reason for hiding this comment

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

👍

@twaugh twaugh merged commit b648bc0 into master Jan 24, 2019
@twaugh twaugh deleted the templates branch January 24, 2019 16:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants