Daniel Shahaf <d...@daniel.shahaf.name> writes:

> Noorul Islam K M wrote on Sat, Feb 05, 2011 at 12:55:55 +0530:
>
>> Daniel Shahaf <d...@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

Reply via email to