Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
> Noorul Islam K M wrote on Sat, Feb 05, 2011 at 12:55:55 +0530:
>
>> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
>> > Noorul Islam K M wrote on Thu, Feb 03, 2011 at 14:15:48 +0530:
>> >> + if (info->depth == svn_depth_exclude)
>> >> + SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
>> >>
>> >
>> > I know that there is already some other place in the code that prints
>> > the Depth: line, why didn't you reuse that place?
>> >
>>
>> It is inside the if block 'if (info->has_wc_info)'. In our case flow
>> will not reach there so I had to put it here.
>>
>
> ... and? Why won't it enter that block? i.e., why is has_wc_info false?
>
> Some of the fields there (eg, checksum and copyfrom) make perfect sense
> even for excluded nodes. (I don't know whether the wc can supply those
> values even for excluded nodes.)
>
I don't think we have much information in DB to be printed.
Below is the values from NODES table for excluded path 'A'
wc_id = 1
local_relpath =
op_depth = 0
parent_relpath =
repos_id = 1
repos_path =
revision = 1
presence = normal
moved_here =
moved_to =
kind = dir
properties = ()
depth = infinity
checksum =
symlink_target =
changed_revision = 1
changed_date = 1297240607634523
changed_author = noorul
translated_size =
last_mod_time =
dav_cache =
file_external =
>> >> + if (tree_conflict || exclude)
>> >> {
>> >> svn_info_t *info;
>> >> svn_error_clear(err);
>> >>
>> >> SVN_ERR(build_info_for_unversioned(&info, pool));
>> >> - info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
>> >>
>> >> + if (tree_conflict)
>> >> + info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
>> >> + else
>> >> + info->depth = svn_depth_exclude;
>> >
>> > s/else/if (exclude)/, please.
>> >
>>
>> There is actually an outer if condition which makes this not useful.
>
> I know (it's about 5 lines above this line). But IMO this way is more
> readable/maintainable.
>
I agree.
>> >> +
>> >> SVN_ERR(svn_wc__node_get_repos_info(&(info->repos_root_URL),
>> >> - NULL,
>> >> + exclude ?
>> >> + &(info->repos_UUID) : NULL,
>> >
>> > Why?
>> >
>>
>> I thought I should not make changes to existing behaviour. I think it is
>> safe to just pass &(info->repos_UUID) in both cases.
>>
>
> ... and? What is the "change to existing behaviour" you're talking about? (I
> guess it's printing the repository UUID for excluded nodes?)
As of now in trunk for this call NULL is passed reference to repository
UUID. I thought I will keep that as such and pass a reference in the
case of excluded. That is why I initially included that condition. Later
I found that it is okay to pass a reference in both cases.
Thanks and Regards
Noorul
Received on 2011-02-09 10:01:03 CET