Noorul Islam K M <noo...@collab.net> writes:

> 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 = 
>

My bad. I pasted wrong information. Here it is.

           wc_id = 1
   local_relpath = A
        op_depth = 0
  parent_relpath = 
        repos_id = 1
      repos_path = A
        revision = 1
        presence = excluded
      moved_here = 
        moved_to = 
            kind = dir
      properties = 
           depth = unknown
        checksum = 
  symlink_target = 
changed_revision = 
    changed_date = 0
  changed_author = 
 translated_size = 
   last_mod_time = 
       dav_cache = 
   file_external = 

Thanks and Regards
Noorul

>>> >> +      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