Added :justify_hash option.
#319
Conversation
|
Didn't have the most fun in the align function, what with indentations, width, and several use cases, but still had some :). The use is fairly coupled with the What helped most is when I started treating |
|
Also noting #317 can be closed when this PR is. |
Allows you to not justify hash key-value pair output. Works for `indent` `> 0` and `< 0` cases. The `= 0` should be unrelated and untouched. Added the specs I thought were needed (the minimum). Stayed within some style checks, and ignored others where deemed appropriate. In addition to working pn the feature per the contributing guidelines: - Went ahead and added myself to the contributor section in CONTRIBUTING, along with fixing some other minor bits. - In CHANGELOG, fixed some minor bits. - Went ahead an added myself here for later release buidl refernce ease. - Also highlighted the fact that my link is to an issue. You can add the PR link to it later.
Pysis868
force-pushed
the
add-justify-hash-option
branch
from
a255d4e to
d043f61
Compare
| end | ||
| else | ||
| value | ||
| end | ||
| end | ||
|
|
||
| def align_justify(value, width) # rubocop:disable Metrics/AbcSize |
There was a problem hiding this comment.
Please remove any rubocop disable comments; They're a bad practice because it'll never report for this metric again.
| def align_justify(value, width) # rubocop:disable Metrics/AbcSize | ||
| if options[:indent] > 0 | ||
| value.rjust(width) | ||
| elsif options[:indent] == 0 |
There was a problem hiding this comment.
These two branches (the elsif/else) can be condensed into just line 142.
| @@ -123,18 +123,29 @@ def outdent | |||
|
|
|||
| def align(value, width) | |||
| if options[:multiline] | |||
There was a problem hiding this comment.
since you are here, it'd be great to resolve the ABCSize issues in this method. Return early unless options[:multiline],
There was a problem hiding this comment.
ok, wasn't sure how far to take Metrics, and I attempted to clear it up some already.
| else | ||
| indent[0, indentation + options[:indent]] + value.ljust(width) | ||
| align_no_justify(value) |
There was a problem hiding this comment.
something about this method name feels off, and it's doing more than just aligning / justifying, sort of-- indenting then justify. Is there a way to break it into those two steps?
|
|
||
| width = left_width(data) if options[:justify_hash] |
There was a problem hiding this comment.
width is now no longer set, which will potentially have unexpected consequences in L49, L51 and/or L56.
There was a problem hiding this comment.
is now potentially no longer set. :)
|
hey @Pysis868 - wanted to follow up on this :) this is a 👍 PR, but i'd like to tune it up a little bit as per the comments. Do you have time to take a look? thanks! 🥇 |
|
I have to switch from awesome print just because of this issue causing me lost time in development. |
Allows you to not justify hash key-value pair output.
Works for
indent> 0and< 0cases. The= 0should be unrelatedand untouched.
Added the specs I thought were needed (the minimum).
Stayed within some style checks, and ignored others where deemed
appropriate.
In addition to working pn the feature per the contributing guidelines:
CONTRIBUTING, along with fixing some other minor bits.
ease.
the PR link to it later.