Skip to content
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

FIXES #176: templates: default table template was wider than terminal #295

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions _t/100basic.t
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,14 @@ EOF
## List all issues, using the table template
###############################################################################


RUNS $jira ls --project BASIC --template table
DIFF <<EOF
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| $(printf %-14s $issue) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| $(printf %-10s $issue) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
EOF

###############################################################################
Expand Down
10 changes: 5 additions & 5 deletions _t/120custom-commands.t
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ EOF

RUNS $jira mine
DIFF <<EOF
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| $(printf %-14s $issue) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| $(printf %-10s $issue) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
EOF
39 changes: 20 additions & 19 deletions _t/130basic-epics.t
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,14 @@ EOF
###############################################################################
## List the issues for the epic
###############################################################################
echo $epic
RUNS $jira epic list $epic

DIFF<<EOF
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
EOF

###############################################################################
Expand All @@ -69,12 +70,12 @@ EOF
RUNS $jira epic list $epic

DIFF<<EOF
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| $(printf %-14s $issue1) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
| $(printf %-14s $issue2) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| $(printf %-10s $issue1) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
| $(printf %-10s $issue2) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
EOF

###############################################################################
Expand All @@ -92,11 +93,11 @@ EOF
RUNS $jira epic list $epic

DIFF<<EOF
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| $(printf %-14s $issue2) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| $(printf %-10s $issue2) | summary | Bug | Medium | To Do | a minute | gojira | gojira |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
EOF

###############################################################################
Expand All @@ -114,8 +115,8 @@ EOF
RUNS $jira epic list $epic

DIFF<<EOF
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+----------------+------------------------------------------+--------------+--------------+--------------+------------+--------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
| Issue | Summary | Type | Priority | Status | Age | Reporter | Assignee |
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
+------------+------------------------------------------+------------+----------------+----------------+------------+---------------------------+--------------+
EOF
28 changes: 22 additions & 6 deletions jiracli/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,14 +293,30 @@ const defaultDebugTemplate = "{{ . | toJson}}\n"
const defaultListTemplate = "{{ range .issues }}{{ .key | append \":\" | printf \"%-12s\"}} {{ .fields.summary }}\n{{ end }}"

const defaultTableTemplate = `{{/* table template */ -}}
{{$w := sub termWidth 107 -}}
+{{ "-" | rep 16 }}+{{ "-" | rep $w }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+{{ "-" | rep 12 }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+
| {{ "Issue" | printf "%-14s" }} | {{ "Summary" | printf (printf "%%-%ds" (sub $w 2)) }} | {{ "Type" | printf "%-12s"}} | {{ "Priority" | printf "%-12s" }} | {{ "Status" | printf "%-12s" }} | {{ "Age" | printf "%-10s" }} | {{ "Reporter" | printf "%-12s" }} | {{ "Assignee" | printf "%-12s" }} |
+{{ "-" | rep 16 }}+{{ "-" | rep $w }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+{{ "-" | rep 12 }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+
{{$issuew := int 10 -}}
{{$typew := int $issuew -}}
{{$priorityw := int 14 -}}
{{$statusw := $priorityw -}}
{{$agew := int 10 -}}
{{$reporterw := int 25 -}}
{{$assigneew := int 12 -}}
{{$dissuew := int (add 2 $issuew) -}}
Copy link
Author

@dobbymoodge dobbymoodge Nov 15, 2019

Choose a reason for hiding this comment

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

add seems to return int64, but the sub template function only wants int type args, so there's just a ton of casting going on.

Something like {{$newval := sub $bar (add 2 $foo) -}} would give an error like ERROR Invalid Usage: template: gojira:1:30: executing "gojira" at <$foo>: wrong type for value; expected int; got int64

{{$dtypew := int (add 2 $typew) -}}
{{$dpriorityw := int (add 2 $priorityw) -}}
{{$dstatusw := int (add 2 $statusw) -}}
{{$dagew := int (add 2 $agew) -}}
{{$dreporterw := int (add 2 $reporterw) -}}
{{$dassigneew := int (add 2 $assigneew) -}}
{{$usedcols := int (add 25 (add $issuew (add $typew (add $priorityw (add $statusw (add $agew (add $reporterw $assigneew))))))) -}}
{{$summaryw := int (max 40 (sub termWidth $usedcols)) -}}
{{$dsummaryw := int (add 2 $summaryw) -}}
+{{ "-" | rep $dissuew }}+{{ "-" | rep $dsummaryw }}+{{ "-" | rep $dtypew }}+{{ "-" | rep $dpriorityw }}+{{ "-" | rep $dstatusw }}+{{ "-" | rep $dagew }}+{{ "-" | rep $dreporterw }}+{{ "-" | rep $dassigneew }}+
| {{ "Issue" | printf (printf "%%-%ds" $issuew) }} | {{ "Summary" | printf (printf "%%-%ds" $summaryw) }} | {{ "Type" | printf (printf "%%-%ds" $typew)}} | {{ "Priority" | printf (printf "%%-%ds" $priorityw) }} | {{ "Status" | printf (printf "%%-%ds" $statusw) }} | {{ "Age" | printf (printf "%%-%ds" $agew) }} | {{ "Reporter" | printf (printf "%%-%ds" $reporterw) }} | {{ "Assignee" | printf (printf "%%-%ds" $assigneew) }} |
+{{ "-" | rep $dissuew }}+{{ "-" | rep $dsummaryw }}+{{ "-" | rep $dtypew }}+{{ "-" | rep $dpriorityw }}+{{ "-" | rep $dstatusw }}+{{ "-" | rep $dagew }}+{{ "-" | rep $dreporterw }}+{{ "-" | rep $dassigneew }}+
{{ range .issues -}}
| {{ .key | printf "%-14s"}} | {{ .fields.summary | abbrev (sub $w 2) | printf (printf "%%-%ds" (sub $w 2)) }} | {{.fields.issuetype.name | printf "%-12s" }} | {{if .fields.priority}}{{.fields.priority.name | printf "%-12s" }}{{else}}<unassigned>{{end}} | {{.fields.status.name | printf "%-12s" }} | {{.fields.created | age | printf "%-10s" }} | {{if .fields.reporter}}{{ .fields.reporter.name | printf "%-12s"}}{{else}}<unassigned>{{end}} | {{if .fields.assignee }}{{.fields.assignee.name | printf "%-12s" }}{{else}}<unassigned>{{end}} |
| {{ .key | printf (printf "%%-%ds" $issuew)}} | {{ .fields.summary | abbrev (int (sub $summaryw 2)) | printf (printf "%%-%ds" $summaryw) }} | {{.fields.issuetype.name | printf (printf "%%-%ds" $typew) }} | {{if .fields.priority}}{{.fields.priority.name | printf (printf "%%-%ds" $priorityw) }}{{else}}<unassigned>{{end}} | {{.fields.status.name | printf (printf "%%-%ds" $statusw) }} | {{.fields.created | age | printf (printf "%%-%ds" $agew) }} | {{if .fields.reporter}}{{ .fields.reporter.name | printf (printf "%%-%ds" $reporterw)}}{{else}}<unassigned>{{end}} | {{if .fields.assignee }}{{.fields.assignee.name | printf (printf "%%-%ds" $assigneew) }}{{else}}<unassigned>{{end}} |
Copy link
Author

Choose a reason for hiding this comment

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

I feel like there must be a better way to get responsive padding than this pattern: {{.fields.issuetype.name | printf (printf "%%-%ds" $typew) }}

I guess we could add a new templating function, something like "padFit": func(max int, content string) string {...}. It would look like abbrev but also handle the case when len(content) <= max, basically moving the printf (printf ... ) logic into the function using fmt.Sprintf() or something...

Copy link
Author

Choose a reason for hiding this comment

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

Then - if I understand the template syntax, which I'm still a bit fuzzy on - we would replace the example I quoted with something like {{.fields.issuetype.name | padFit $typew }}, which seems like a win

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this is pretty rough, your solution is probably the best with what we have. I wonder if we should just add some template functions that correspond to https://github.com/olekukonko/tablewriter to make the table writing easier.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @dobbymoodge, I played a bit more with tablewriter, I think it will work better than this manual craziness that I started with the manual table formatting. For a proof of concept I was able to write this template:

const defaultTableTemplate = `
{{- headers "Issue" "Summary" "Type" "Priority" "Status" "Age" "Reporter" "Assignee" -}}
{{- range .issues -}}
  {{- row -}}
    {{- cell .key -}}
    {{- cell .fields.summary -}}
    {{- cell .fields.issuetype.name -}}
    {{- if .fields.priority -}}
      {{- cell .fields.priority.name -}}
    {{- else -}}
      {{- cell "<none>" -}}
    {{- end -}}
    {{- cell .fields.status.name -}}
    {{-  cell (.fields.created | age) -}}
    {{- if .fields.reporter -}}
      {{- cell .fields.reporter.name -}}
    {{- else -}}
      {{- cell "<unknown>" -}}
    {{- end -}}
    {{- if .fields.assignee -}}
      {{- cell .fields.assignee.name -}}
    {{- else -}}
      {{- cell "<unassigned>" -}}
    {{- end -}}
{{- end -}}
`

and I get this now:

+------------+---------+------+----------+--------+---------+----------+----------+
|   ISSUE    | SUMMARY | TYPE | PRIORITY | STATUS |   AGE   | REPORTER | ASSIGNEE |
+------------+---------+------+----------+--------+---------+----------+----------+
| BASIC-3679 | summary | Bug  | Medium   | To Do  | 2 hours | gojira   | gojira   |
+------------+---------+------+----------+--------+---------+----------+----------+

It auto sizes the columns based on the text, otherwise it will wrap cells to fit within the console width.

So I think I will continue with this approach as the templates seem to be much more readable and will create a PR for it and ping you.

{{ end -}}
+{{ "-" | rep 16 }}+{{ "-" | rep $w }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+{{ "-" | rep 12 }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+
+{{ "-" | rep $dissuew }}+{{ "-" | rep $dsummaryw }}+{{ "-" | rep $dtypew }}+{{ "-" | rep $dpriorityw }}+{{ "-" | rep $dstatusw }}+{{ "-" | rep $dagew }}+{{ "-" | rep $dreporterw }}+{{ "-" | rep $dassigneew }}+
`
const defaultAttachListTemplate = `{{/* table template */ -}}
+{{ "-" | rep 12 }}+{{ "-" | rep 30 }}+{{ "-" | rep 12 }}+{{ "-" | rep 14 }}+{{ "-" | rep 14 }}+
Expand Down