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

Parameterize the max line length for the report table. #30

Closed
wants to merge 1 commit into from

Conversation

sjudd
Copy link
Contributor

@sjudd sjudd commented Jan 11, 2018

The motivation is to optionally provide better formatting when printing to output that has strict line limits. In my case I'm particularly interested in the output in travis-ci's log. I've attached a screenshot that shows the current behavior when Travis' log line length is exceeded.
screen shot 2018-01-10 at 6 27 22 pm

This could be more intelligent, but for now this just restricts the length of the message column. I've retained the existing behavior of allowing up to 100 characters in the message column if no line limit is specified.

    
This adds an extra iteration across all violations in all cases. It’s 
possible to only do so when the max line length is specified, but doing 
so increases the number of branches and makes the code a bit harder to
read.

This also creates the FlipTable twice, once to measure the line lengths 
and then again after we’ve set our target line breaks. It’s also 
possible to make some assumptions about the whitespace and number of 
characters that FlipTable will add and avoid the second table creation. 
However, assuming that the construction of the table is not super 
performance critical, it seems more robust to just measure the output.

A nice TODO might be to add coherent line breaking so we don’t break
in the middle of words, but this isn’t technically a regression from
the current 100 character message limit.
@tomasbjerre
Copy link
Owner

The Rule and Message columns seems to be the most common source to these problems...

Maby the better (simpler) solution is to supply a fixed limit to all 4 columns? And just apply the addNewlines method to all columns.

@tomasbjerre
Copy link
Owner

Released now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants