Refactor GM to separate config parsing, attempt 2

Refactor GM to separate config parsing, attempt 2

Committed: http://code.google.com/p/skia/source/detail?r=10724

[ducky, 2013-08-14 18:37:47]

Here's attempt 2.
The changes are:
- GM config parsing ordering changed so that configs displayed before
expectations path
- Rebaseline for new simpler config parsing - it no longer checks for the last
element - so there's a trailing (invisible) space in the output. But the code is
a lot cleaner.

[vandebo (ex-Chrome), 2013-08-14 18:47:21]

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp#newcode1863
gm/gmmain.cpp:1863: SkString configStr("These configs will be run: ");
Why not omit the space at the end of this line and put the space at the
beginning of the string inside the while loop?  That will preserve the output
and still simplify the code.

[epoger, 2013-08-14 18:48:31]

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp#newcode1863
gm/gmmain.cpp:1863: SkString configStr("These configs will be run: ");
On 2013/08/14 18:47:21, vandebo wrote:
> Why not omit the space at the end of this line and put the space at the
> beginning of the string inside the while loop?  That will preserve the output
> and still simplify the code.

Clever, I like it.

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp#newcode2035
gm/gmmain.cpp:2035: !parse_flags_gmmain_paths(&gmmain)) {
Am I correct in assuming that patchset 1 is just the code exactly as you had
landed it previously?  If so, I won't bother looking at patchset 1 at all, just
the changes from patchset 1 to patchset 2.

Pretty cool that this is the only change required.

[ducky, 2013-08-14 18:50:33]

This should be cleaner.

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp#newcode1863
gm/gmmain.cpp:1863: SkString configStr("These configs will be run: ");
On 2013/08/14 18:47:21, vandebo wrote:
> Why not omit the space at the end of this line and put the space at the
> beginning of the string inside the while loop?  That will preserve the output
> and still simplify the code.

Good point. Will do.

[vandebo (ex-Chrome), 2013-08-14 18:56:38]

Fixes LGTM

[epoger, 2013-08-14 18:59:21]

LGTM!

So I guess the full set of changes (compared to the initial attempt) was just
these:

https://codereview.chromium.org/23072014/diff2/1:25001/gm/gmmain.cpp

Nice!

[ducky, 2013-08-14 19:00:44]

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp
File gm/gmmain.cpp (right):

https://codereview.chromium.org/23072014/diff/3001/gm/gmmain.cpp#newcode2035
gm/gmmain.cpp:2035: !parse_flags_gmmain_paths(&gmmain)) {
On 2013/08/14 18:48:31, epoger wrote:
> Am I correct in assuming that patchset 1 is just the code exactly as you had
> landed it previously?  If so, I won't bother looking at patchset 1 at all,
just
> the changes from patchset 1 to patchset 2.
> 
> Pretty cool that this is the only change required.

It should be - so only changes after patchset 1 are new.
Since this was just a refactor GM, the functionality was kept mostly the same.
Some things ended up re-ordered and there were some invisible-to-humans changes,
but I guess the bots didn't like those.

[commit-bot: I haz the power, 2013-08-14 21:43:26]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/richardlin@chromium.org/23072014/25001

[commit-bot: I haz the power, 2013-08-14 21:56:49]

Change committed as 10724