Bug 39208 – webkit-patch needs --verbose flag to enable DEBUG logging

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

webkit-patch needs --verbose flag to enable DEBUG logging

[Eric Seidel (no email)]

Created attachment 56222 [details]
Patch

[Chris Jerdonek]

Comment on attachment 56222 [details]
Patch

Some thoughts to consider prior to landing.  r=me

> +    is_verbose = "-v" in sys.argv or "--verbose" in sys.argv

I wonder if there is a nice way to avoid writing sys.argv twice -- e.g.
Python set intersection?

> +    def _command_for_printing(self, args):
> +        """Used for producing a print-ready string representing command args.
> +        The string should be copy/paste ready for execution in a shell."""

PEP8 says the doc string for functions should begin with an imperative
word, e.g. "Return a print-ready string...."

PEP8 also has pretty specific rules about the formatting of doc strings.  This
one would look something like this--

    def _command_for_printing(self, args):
        """Return a print-ready string that represents command args.

        The string should be copy/paste ready for execution in a shell.
        
        """

> +            if isinstance(arg, unicode):
> +                # Escape any non-ascii characters for easy copy/paste
> +                arg = arg.encode("unicode_escape")
> +            # quotes?

What does the "quotes?" comment mean -- is that a FIXME?

>  
> +        _log.debug("\"%s\" took %.2fs" % (self._command_for_printing(args), time.time() - start_time))

I would use single quotes to avoid escaping the double quotes.

[Eric Seidel (no email)]

Addressed Chris's comments.  landing.

[Eric Seidel (no email)]

Committed r60066: <http://trac.webkit.org/changeset/60066>