[Webkit-unassigned] [Bug 123185] Refactor option parsing in WebKitTestRunner

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=123185





--- Comment #8 from Peter Gal <galpeter at inf.u-szeged.hu>  2013-11-21 01:32:57 PST ---
(From update of attachment 215645)
View in context: https://bugs.webkit.org/attachment.cgi?id=215645&action=review

> Tools/WebKitTestRunner/Options.cpp:34
> +Options::Options(double dlto, double dsto):

What does dlto/dsto mean? If possible we should not use abbreviations.

> Tools/WebKitTestRunner/Options.cpp:48
> +bool handleOptionTimeout(Options& o, const char* opt, const char* arg)

ditto.

> Tools/WebKitTestRunner/Options.cpp:56
> +bool handleOptionNoTimeout(Options& o, const char* opt, const char* arg)

ditto

> Tools/WebKitTestRunner/Options.cpp:62
> +bool handleOptionNoTimeoutAtAll(Options& o, const char* opt, const char* arg)

ditto

> Tools/WebKitTestRunner/Options.cpp:69
> +bool handleOptionVerbose(Options& o, const char* opt, const char* arg)

ditto.

> Tools/WebKitTestRunner/Options.cpp:75
> +bool handleOptionGcBetweenTests(Options& o, const char* opt, const char* arg)

ditto.

> Tools/WebKitTestRunner/Options.cpp:81
> +bool handleOptionPixelTests(Options& o, const char* opt, const char* arg)

ditto.

> Tools/WebKitTestRunner/Options.cpp:87
> +bool handleOptionPrintXSupportedFeatures(Options& o, const char* opt, const char* arg)

ditto.

> Tools/WebKitTestRunner/Options.cpp:101
> +OptionsHandler::OptionsHandler(Options& o): options(o)

ditto.

> Tools/WebKitTestRunner/Options.cpp:103
> +    add(Option("--timeout",                  "Sets long timeout to <param> and scales short timeout.", handleOptionTimeout, true));

There are too many whitespace before the option description, we should keep only one.

> Tools/WebKitTestRunner/Options.cpp:128
> +    for (int argCounter = 1; argCounter < argc; argCounter++) {

++argCounter instead of argCounter++.

> Tools/WebKitTestRunner/Options.cpp:133
> +        for (Option &o : optionlist) {

don't use abbreviations.
'Option&' instead of 'Option &'

> Tools/WebKitTestRunner/Options.cpp:153
> +void OptionsHandler::help(FILE *channel)

'FILE*' instead of the 'FILE *'

> Tools/WebKitTestRunner/Options.cpp:170
> +void OptionsHandler::add(Option o)

Do we need this method at all? We only use it in the handler constructor. On the side note: Maybe we can use Option& here to avoid copy.

> Tools/WebKitTestRunner/Options.h:59
> +    const char* desc;

desc-> description.

> Tools/WebKitTestRunner/Options.h:74
> +    static const char * usagestr;
> +    static const char * helpstr;

maybe we can remove the str part from the variable name, and 'char*' instead of 'char *'.

> Tools/WebKitTestRunner/TestController.cpp:298
> +    m_longTimeout                 = wtrOptions.m_longTimeout;
> +    m_shortTimeout                = wtrOptions.m_shortTimeout;
> +    m_useWaitToDumpWatchdogTimer  = wtrOptions.m_useWaitToDumpWatchdogTimer;
> +    m_forceNoTimeout              = wtrOptions.m_forceNoTimeout;
> +    m_verbose                     = wtrOptions.m_verbose;
> +    m_gcBetweenTests              = wtrOptions.m_gcBetweenTests;
> +    m_shouldDumpPixelsForAllTests = wtrOptions.m_shouldDumpPixelsForAllTests;
> +    m_paths                       = wtrOptions.m_paths;

keep only one space before the = signs.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.