[Webkit-unassigned] [Bug 119618] Adding a .ycm_extra_conf file for webkitGtk

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mrobinson at webkit.org




--- Comment #5 from Martin Robinson <mrobinson at webkit.org>  2013-12-12 15:16:20 PST ---
(In reply to comment #4)
> (From update of attachment 219016 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219016&action=review
> 
> LGTM, with a small refactoring proposal for the first function
> 
> > Tools/gtk/ycm_extra_conf.py:47
> > +            for flag in FLAGS_PRECEDING_PATHS:
> 
> This for block could be replaced by if argument in FLAGS_PRECEDING_PATHS, you could always ensure argument is either a flag or a path to have a single code path here, it will be much more readable.

The arguments may be of the form -I /path or --argument=path. In the former case, we have to iterate the list of arguments to do a bunch of string searches. I did consider breaking out the  argument in FLAGS_PRECEDING_PATHS case like this:

for argument in arguments:
    if make_next_absolute:
        ...
    elif argument in FLAGS_PRECEDING_PATHS:
        make_next_absolute = True
    else:
        for flag in FLAGS_PRECEDING_PATHS:
            if argument.startswith(flag):
                ...

I went with the current approach because it was a little bit more efficient. I like both approaches though, so I can use whichever you prefer. :)

> 
> > Tools/gtk/ycm_extra_conf.py:54
> > +                # Some argument contain the flag and the path together. For these
> 
> Missing last bit of the comment?

Yes! Thanks.

> 
> > Tools/gtk/ycm_extra_conf.py:101
> > +        print "==== Error during reading %s file", trace_file_path
> 
> s/during/while/?

Thanks. I missed this.

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