Re: [PATCH v7 03/11] ref-filter: add option to pad atoms to the right
flat view
From: Karthik Nayak <hidden>
Date: 2016-06-15 23:06:03
On Thu, Jul 30, 2015 at 11:26 PM, Eric Sunshine [off-list ref] wrote:
In an earlier review, Matthieu pointed out that this test failed to
ensure that the 'padright' value did not leak into the next atom. In a
subsequent version, you fixed the test to check that condition, but
now you've somewhat lost it again, at least visually. That is, because
whitespace is "invisible" and because 'padright' now also affects
literal strings, someone reading this test can't tell if those
trailing |'s in the expected output are padded or not. You could use a
different format string to prove that the 'padright' value doesn't
leak. For instance:
%(padright:10)%(refname:short)|%(padright:5)|%(refname:short)
This way, as long as the two |'s are side-by-side, then you've proved
that the first one wasn't affected by the preceding 'padright:10'. You
could also add back the %(refname:short) at the front of the pattern,
as you currently have it, if you want to prove that padding is not
enabled at the start of format.Thanks Eric. This seems like a better improvement over my test. Will implement this :) -- Regards, Karthik Nayak