Re: [PATCH] Add size information to svn_client_info()

Daniel Rall wrote:

>> Index: subversion/libsvn_client/info.c
>> ===================================================================
>> --- subversion/libsvn_client/info.c (revision 25433)
>> +++ subversion/libsvn_client/info.c (working copy)
>> @@ -54,6 +54,7 @@
>> tmpinfo->last_changed_author = dirent->last_author;
>> tmpinfo->lock = lock;
>> tmpinfo->depth = svn_depth_unknown;
>> + tmpinfo->size = dirent->size;
>
> It appears that we need to set tmpinfo->working_size to
> SVN_WC_ENTRY_WORKING_SIZE_UNKOWN.

of course, you're right.

>> *info = tmpinfo;
>> return SVN_NO_ERROR;
>> @@ -92,6 +93,8 @@
>> tmpinfo->conflict_wrk = entry->conflict_wrk;
>> tmpinfo->prejfile = entry->prejfile;
>> tmpinfo->changelist = entry->changelist;
>> + tmpinfo->working_size = entry->working_size;
>> + tmpinfo->size = SVN_INFO_SIZE_UNKNOWN;
>>
>> /* lock stuff */
>> if (entry->lock_token) /* the token is the critical bit. */
>
> I tweaked the doc strings as well (revised patch attached).
>
> I find the bare "size" variable name a little confusing, since for a
> directory I'd expect that to list the number of children or something
> else useful. Perhaps "repos_file_size" and "working_file_size" would
> be more appropriate? Or are we leaving the door open to re-purpose
> these fields at a later time?

I think 'size' should be enough. For directories it it 0 as the docs
will indicate. I don't think that people would expect 'size' to be the
number of files inside a folder - it would then be called 'children' or
something like that.
And since there's also a flag indicating whether the entry is for a file
or folder, a value of 0 won't be confusing.

> I think this patch addresses all of the issues raised by glasser,
> except for:
>
> stefan wrote:
>> glasser wrote:
>>> Should these be printed in "svn info"?
>> I can't really say, I'm not using the CL client that often. I'm also not
>> sure if you have some naming conventions for the xml output, so I've
>> left that one out.
>
> AIUI, the command-line client would display either the repository file
> size, or the translated WC file size, but not both. I don't see
> display of this variable file size field as a compelling advantage for
> the command-line UI, but nor would I be adverse to seeing it added.

I can't really comment on this. I'm not the one who uses the CL client
very often, and usually only for some basic commands or to test
something. Which means I better leave that discussion up to others
better suited than me.

But if there's nothing more (apart from the command line UI) holding
this patch back, I would really appreciate it if someone could commit
it. I really need the size information here for some drag-n-drop
operations inside the shell.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org