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

Fixed Bug #1042 Snapctl error message should truncate #1165

Merged
merged 1 commit into from
Aug 30, 2016

Conversation

kjlyon
Copy link
Contributor

@kjlyon kjlyon commented Aug 29, 2016

Fixes #1042

Summary of changes:

  • Long error messages will not wrap any more unless --verbose is specified
  • Error will display until edge of terminal window and end with '...'
  • If terminal is large enough for whole error message or --verbose is specified, whole error message will be shown

Testing done:

  • Tested with mock-file.yaml and mock.go to display error messages of varying length

@intelsdi-x/snap-maintainers

Error message size adjusts based on size of terminal so that it does not wrap.

@@ -477,6 +480,10 @@ func listTask(ctx *cli.Context) error {
"LAST FAILURE",
)
for _, task := range tasks.ScheduledTasks {
lastErrorMessage := task.LastFailureMessage
if !ctx.Bool("verbose") && len(lastErrorMessage)+153 > termWidth && termWidth > 164 {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the significance of 153 and 164?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

154 is the number of characters in the output before the error:
61b55838-0605-430c-a209-2f3328880141 Task-61b55838-0605-430c-a209-2f3328880141 Running 13 0 6 4:17PM 8-29-2016 error is printed here

164 is 154 + "LAST FAILURE". So that if the title bar wraps then the message will wrap too. If the title bar doesn't wrap, then the error will truncate.

Copy link
Contributor

@IRCody IRCody Aug 29, 2016

Choose a reason for hiding this comment

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

I think adding a comment to say what they mean could be good so that in the future if the format of the table changes we will know to update this also.

It also seems to work if the task name is shorter than the default one which was my other concern here.

@@ -477,22 +481,39 @@ func listTask(ctx *cli.Context) error {
"LAST FAILURE",
)
for _, task := range tasks.ScheduledTasks {
lastErrorMessage := task.LastFailureMessage
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed anymore. Probably makes sense to just pass task.LastFailureMessage in directly.

Error message size adjusts based on size of terminal so that it does not wrap.
@IRCody
Copy link
Contributor

IRCody commented Aug 30, 2016

LGTM

@candysmurf candysmurf merged commit d3629a9 into intelsdi-x:master Aug 30, 2016
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