On Thu, Jul 30, 2015 at 11:26 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html