[PATCH] v3. replace alls svn_wc_entry() in libsvn_client/patch.c

[Daniel Näslund <da...@longitudo.com>]

On Tue, Aug 18, 2009 at 12:17:30PM -0500, Hyrum K. Wright wrote:
> On Aug 18, 2009, at 12:12 PM, Julian Foad wrote:
> 
> > Replacing one line of code with a long block, eight times
> > over, cannot be a good thing. Can you make a replacement function that
> > does exactly what we need in one line, and so avoid this code bloat?
> > Even a local helper function in this file would be an improvement,  
> > but I
> > assume this same pattern will be repeated in many files so a new
> > svn_wc__... function would be best.
> 
> My inner self is saying the same thing here.   
> svn_wc__get_entry_versioned() was intentioned to replace  
> svn_wc__entry_versioned(), and we've found a few places where we can  
> get away with using it in place of svn_wc_entry(), but a modified  
> helper would be nice at this point.

My bad. I was assumning that since this was a temporary solution I could
get away with a long nasty repeating block of code. 

I call the new function svn_wc__maybe_get_entry(). I guess I've seen
better names but I couldn't come up with anything else! 

> >> [[[
> >> Replaced svn_wc_entry() with svn_wc__get_entry_versioned(). Catch
> >> SVN_ERR_ENTRY_NOT_FOUND since it is not expected.
> >
> > I know you know why you're doing this, but tell us in the log message:
> >
> >  As part of getting rid of adm_access batons for WC-NG, replace
> >  some calls to svn_wc_entry() with svn_wc__get_entry_versioned().
> >

Point taken. Trying this instead:

[[[
As part of getting rid of adm_access batons for WC-NG, replace some
calls to svn_wc_entry() with svn_wc__maybe_get_entry().

* subversion/include/private/svn_wc_private.h
  (svn_wc__maybe_get_entry): Declare.

* subversion/libsvn_wc/entries.c
  (svn_wc__maybe_get_entry): New. Wrapper for
    svn_wc__get_entry_versioned() with error and return semantics like
    svn_wc_entry().

* subversion/libsvn_client/patch.c
  (merge_file_changed, merge_file_added, merge_file_deleted,
  merge_dir_added, init_patch_target): Replaced svn_wc_entry() with
    svn_wc__maybe_get_entry(). Use absolute paths.
]]]

Two test failures (also on trunk):
switch_tests 18 21

Mvh
Daniel

Re: [PATCH] v3. replace alls svn_wc_entry() in libsvn_client/patch.c

["Hyrum K. Wright" <hy...@hyrumwright.org>]

On Aug 19, 2009, at 9:14 AM, Daniel Näslund wrote:

> On Tue, Aug 18, 2009 at 12:17:30PM -0500, Hyrum K. Wright wrote:
>> On Aug 18, 2009, at 12:12 PM, Julian Foad wrote:
>>
>>> Replacing one line of code with a long block, eight times
>>> over, cannot be a good thing. Can you make a replacement function  
>>> that
>>> does exactly what we need in one line, and so avoid this code bloat?
>>> Even a local helper function in this file would be an improvement,
>>> but I
>>> assume this same pattern will be repeated in many files so a new
>>> svn_wc__... function would be best.
>>
>> My inner self is saying the same thing here.
>> svn_wc__get_entry_versioned() was intentioned to replace
>> svn_wc__entry_versioned(), and we've found a few places where we can
>> get away with using it in place of svn_wc_entry(), but a modified
>> helper would be nice at this point.
>
> My bad. I was assumning that since this was a temporary solution I  
> could
> get away with a long nasty repeating block of code.
>
> I call the new function svn_wc__maybe_get_entry(). I guess I've seen
> better names but I couldn't come up with anything else!

I couldn't think of anything better, so I figured I'd commit the patch  
and then bikeshed about the name later. I tweaked a couple of things  
like whitespace in function declarations, and added a missing pointer  
dereference.  Committed in r38893.

-Hyrum

>>>> [[[
>>>> Replaced svn_wc_entry() with svn_wc__get_entry_versioned(). Catch
>>>> SVN_ERR_ENTRY_NOT_FOUND since it is not expected.
>>>
>>> I know you know why you're doing this, but tell us in the log  
>>> message:
>>>
>>> As part of getting rid of adm_access batons for WC-NG, replace
>>> some calls to svn_wc_entry() with svn_wc__get_entry_versioned().
>>>
>
> Point taken. Trying this instead:
>
> [[[
> As part of getting rid of adm_access batons for WC-NG, replace some
> calls to svn_wc_entry() with svn_wc__maybe_get_entry().
>
> * subversion/include/private/svn_wc_private.h
>  (svn_wc__maybe_get_entry): Declare.
>
> * subversion/libsvn_wc/entries.c
>  (svn_wc__maybe_get_entry): New. Wrapper for
>    svn_wc__get_entry_versioned() with error and return semantics like
>    svn_wc_entry().
>
> * subversion/libsvn_client/patch.c
>  (merge_file_changed, merge_file_added, merge_file_deleted,
>  merge_dir_added, init_patch_target): Replaced svn_wc_entry() with
>    svn_wc__maybe_get_entry(). Use absolute paths.
> ]]]
>
> Two test failures (also on trunk):
> switch_tests 18 21
>
> Mvh
> Daniel
> <remove_entry_patch_v3.diff>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2385794