Use strategic newlines to improve rule output readability #211
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `github.com/olekukonko/tablewriter` package used for formatting of the rule output does not correctly handle text
with explicit (as opposed by ones added during auto-wrapping) line breaks.
With reflow enabled, these explicit line breaks are removed and the line wrapped solely based on the column width, just
as it would be without the explicit line breaks. For example, given this rule message:
```
.exe file(s) found. Presence of these files blocks addition to the Library Manager index:
deleteme-testdata\bar.exe
deleteme-testdata\examples\baz.exe
deleteme-testdata\examples\qux\asdf.exe
(Rule LS007)
```
The output is:
```
ERROR: .exe file(s) found. Presence of these files blocks addition to the Library
Manager index: deleteme-testdata/bar.exe deleteme-testdata/examples/baz.exe
deleteme-testdata/examples/qux/asdf.exe (Rule LS007)
```
With reflow disabled, each explicit line break becomes a blank line:
```
ERROR: .exe file(s) found. Presence of these files blocks addition to the Library Manager index:
deleteme-testdata/bar.exe
deleteme-testdata/examples/baz.exe
deleteme-testdata/examples/qux/asdf.exe
(Rule LS007)
```
I could not find any way to coerce the package to produce the desired output:
```
ERROR: .exe file(s) found. Presence of these files blocks addition to the Library Manager index:
deleteme-testdata/bar.exe
deleteme-testdata/examples/baz.exe
deleteme-testdata/examples/qux/asdf.exe
(Rule LS007)
```
so I had to resort to post-processing of the non-reflowed output to remove the spurious blank lines.
Readability of rule message output may be improved by inserting strategic line breaks. In this case, the current behavior of appending the rule ID suffix to the non-verbose rule violation output on the same line will not always be appropriate. So when the rule message contains explicit line breaks, the rule ID should be appended on a new line, with the previous behavior preserved in the case of single line rule messages (which will be wrapped according to the column size).
This rule is for packages[].name value of "arduino". Since the identifier for packages is the name value and the name is already stated in the rule message, identifying the package in violation of the rule by its ID is senseless.
In the cases where a project may violate a rule multiple times, the rule message contains a list of identifiers for the violations. Previously, this always took the form of a comma-separated list. These identifiers can sometimes be quite long. When automatic line wrapping occurred in the middle of an identifier, it could make the message difficult to interpret, especially now that Arduino Lint wraps its own output in a tabular form. The solution is to insert line breaks into the rule message where appropriate. This is done for all violation identifier lists that are likely to have long identifiers, as well as additional locations as needed. The rule ID is appended to the rule message and there are also situations where the rule message structure will contain other text after the violation identifier list. For this reason, the list is indented to visually differentiate it from the non-list message text that follows the list.
per1234
added
type: enhancement
Codecov Report
@@ Coverage Diff @@
## main #211 +/- ##
==========================================
+ Coverage 87.96% 87.97% +0.01%
==========================================
Files 43 43
Lines 4170 4176 +6
==========================================
+ Hits 3668 3674 +6
Misses 391 391
Partials 111 111
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
silvanocerza
approved these changes
Jul 26, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the cases where a project may violate a rule multiple times, the rule message contains a list of identifiers for the
violations. Previously, this always took the form of a comma-separated list. These identifiers can sometimes be quite
long. When automatic line wrapping occurred in the middle of an identifier, it could make the message difficult to
interpret, especially now that Arduino Lint wraps its own output in a tabular form.
The solution is to insert line breaks into the rule message where appropriate. This is done for all violation identifier
lists that are likely to have long identifiers, as well as additional locations as needed.
The rule ID is appended to the rule message and there are also situations where the rule message structure will contain
other text after the violation identifier list. For this reason, the list is indented to visually differentiate it from
the non-list message text that follows the list.
Example output before:
after: