[cfe-commits] [PATCH] [cindex.py] Allow to configure the path of libclang

[Tobias Grosser]

On 09/03/2012 07:29 PM, Gregory Szorc wrote:
> I like it! This is much needed.
>
> My initial thoughts were that the Config class is a bit heavyweight. IMO
> a module-level function to force reload the global lib instance to point
> to a different library would be sufficient. But, thinking it through,
> I'm not sure how this could be done without making things weird. The
> best I can think of is that at module load time it would attempt to find
> a library. If it doesn't, lib is None and the first use of functionality
> results in an opaque message about None not having some attribute. If
> someone wished to point to a non-default path, they could import the
> module and call a function which replaced the module variable "lib."
> But, this is essentially what you have coded in the Config class. So, I
> guess the extra complexity is warranted. That being said, I'm not
> convinced a class is needed (a few module-level functions and variable
> would suffice, IMO). But, I don't feel too strongly, so LGTM.

Hi Gregory,

thanks for the review. I was myself thinking if an entire class for this 
is worth it. There have been two reasons I decided to go for it:

   - It can group further options that may be added in the future
   - We can provide better error messages

The second point was probably not clear in my patch. I committed a 
slightly modified version, which now points to Config.set_library_path()
in case libclang can not be found. Nothing fancy, but it may be useful. 
And more important, we will not have random 'None not having attribute' 
messages, but we can decide ourselves which error message to send.

I committed the slightly changed version in 163121.

Tobi