[cfe-commits] [PATCH 1/2] [clang.py] Add TranslationUnit.get_{file, source_location, source_range}

[Gregory Szorc]

Committed r160107
     M    bindings/python/tests/cindex/test_translation_unit.py
     M    bindings/python/clang/cindex.py

On 7/10/12 6:23 AM, Manuel Klimek wrote:
> On Mon, Jul 9, 2012 at 8:27 AM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
>> On 7/1/12 11:03 PM, Manuel Klimek wrote:
>>
>> High-level remark:
>> The idea of having multiple different parameters of which only one must ever
>> be set seems strange to me. Why not instead have multiple methods with
>> different names? I'd also prefer having 1 parameter and finding out what
>> methods it provides.
>>
>> Changed API to single argument with type detection. I don't like having
>> multiple methods with different names because that feels too "Java-y" for
>> me. I prefer a single method that just works and accepts whatever is thrown
>> at it.
> I'm fine with this style for python... It seems to be what the crowd craves ;)
>
> LGTM
>
>>
>>
>>
>> On Sat, Jun 30, 2012 at 4:15 AM, Gregory Szorc <gregory.szorc at gmail.com>
>> wrote:
>>> Updated patch.
>>>
>>> On Fri, Jun 29, 2012 at 8:47 AM, Gregory Szorc <gregory.szorc at gmail.com>
>>> wrote:
>>>> Having thought about this in my sleep, I may want to rescind this
>>>> review request and refactor things a little.
>>>>
>>>> 1) I may change "range" and "source_range" names to "extent" since
>>>> that is what is used elsewhere.
>>>> 2) I may combine the bounds to obtain ranges/extents from two
>>>> arguments to 2-tuples.
>>>> 3) I may remove the "_source" from get_source_location and
>>>> get_source_extent. I don't think that's any less clear.
>>>>
>>>> This will of course invalidate the token API patch that followed,
>>>> albeit trivially.
>>>>
>>>> On Fri, Jun 29, 2012 at 12:13 AM, Gregory Szorc
>>>> <gregory.szorc at gmail.com> wrote:
>>>>> These are just convenience APIs to make obtaining File,
>>>>> SourceLocation, and SourceRange instances easier.
>>>>>
>>>>> Old way:
>>>>>
>>>>> f = File.from_name(tu, 'foo.c')
>>>>> location = SourceLocation.from_offset(tu, f, 10)
>>>>>
>>>>> New way:
>>>>>
>>>>> location = tu.get_source_location('foo.c', offset=10)
>>
>>