Re: [PATCH v3 00/12] get_short_oid UI improvements

2018-05-03 Thread Jacob Keller
On Wed, May 2, 2018 at 6:45 AM, Derrick Stolee  wrote:
> On 5/2/2018 8:42 AM, Derrick Stolee wrote:
>>
>> On 5/1/2018 2:40 PM, Ævar Arnfjörð Bjarmason wrote:
>>>
>>> The biggest change in v3 is the no change at all to the code, but a
>>> lengthy explanation of why I didn't go for Derrick's simpler
>>> implementation. Maybe I'm wrong about that, but I felt uneasy
>>> offloading undocumented (or if I documented it, it would only be for
>>> this one edge-case) magic on the oid_array API. Instead I'm just
>>> making this patch a bit more complex.
>>
>>
>> I think that's fair. Thanks for going along with me on the thought
>> experiment.
>
>
> Also, v3 looks good to me.
>
> Reviewed-by: Derrick Stolee 
>

I also reviewed this, and it looks good to me as well.

Thanks,
Jake


Re: [PATCH v3 00/12] get_short_oid UI improvements

2018-05-02 Thread Derrick Stolee

On 5/2/2018 8:42 AM, Derrick Stolee wrote:

On 5/1/2018 2:40 PM, Ævar Arnfjörð Bjarmason wrote:

The biggest change in v3 is the no change at all to the code, but a
lengthy explanation of why I didn't go for Derrick's simpler
implementation. Maybe I'm wrong about that, but I felt uneasy
offloading undocumented (or if I documented it, it would only be for
this one edge-case) magic on the oid_array API. Instead I'm just
making this patch a bit more complex.


I think that's fair. Thanks for going along with me on the thought 
experiment.


Also, v3 looks good to me.

Reviewed-by: Derrick Stolee 



Re: [PATCH v3 00/12] get_short_oid UI improvements

2018-05-02 Thread Derrick Stolee

On 5/1/2018 2:40 PM, Ævar Arnfjörð Bjarmason wrote:

The biggest change in v3 is the no change at all to the code, but a
lengthy explanation of why I didn't go for Derrick's simpler
implementation. Maybe I'm wrong about that, but I felt uneasy
offloading undocumented (or if I documented it, it would only be for
this one edge-case) magic on the oid_array API. Instead I'm just
making this patch a bit more complex.


I think that's fair. Thanks for going along with me on the thought 
experiment.


-Stolee


[PATCH v3 00/12] get_short_oid UI improvements

2018-05-01 Thread Ævar Arnfjörð Bjarmason
Comments inline:

Ævar Arnfjörð Bjarmason (12):
  sha1-name.c: remove stray newline

No changes.

  sha1-array.h: align function arguments

Mention the correct commit to blame for disalignment in the commit
message, and also fix it in the *.c file.

  git-p4: change "commitish" typo to "committish"

No changes.

  cache.h: add comment explaining the order in object_type

Trivial commit message rewording.

  sha1-name.c: move around the collect_ambiguous() function

No changes.

  get_short_oid: sort ambiguous objects by type, then SHA-1

The biggest change in v3 is the no change at all to the code, but a
lengthy explanation of why I didn't go for Derrick's simpler
implementation. Maybe I'm wrong about that, but I felt uneasy
offloading undocumented (or if I documented it, it would only be for
this one edge-case) magic on the oid_array API. Instead I'm just
making this patch a bit more complex.

  get_short_oid: learn to disambiguate by ^{tag}
  get_short_oid: learn to disambiguate by ^{blob}
  get_short_oid / peel_onion: ^{tree} should be tree, not treeish
  get_short_oid / peel_onion: ^{commit} should be commit, not committish
  config doc: document core.disambiguate
  get_short_oid: document & warn if we ignore the type selector

No changes except one trivial commit message formatting fix.

 Documentation/config.txt  | 17 +
 Documentation/technical/api-oid-array.txt | 17 +++--
 cache.h   | 13 +++-
 git-p4.py |  6 +-
 sha1-array.c  | 21 +-
 sha1-array.h  |  7 +-
 sha1-name.c   | 80 +++
 t/t1512-rev-parse-disambiguation.sh   | 58 +---
 8 files changed, 184 insertions(+), 35 deletions(-)

-- 
2.17.0.290.gded63e768a