You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@subversion.apache.org by Mukund <mu...@tessna.com> on 2003/07/09 12:03:54 UTC

[PATCH] Better progress indicator for the 'svn' command-line client

Hi all

The attached patch contains a better deltas transfer progress indicator for
the 'svn' command-line client. A visual example of what it does can be found
here: http://www.mukund.org/temp/svn.png

Due to the nature of the patch, the notification API had to be slightly
modified (svn_wc_notify_func_t), resulting in changes to a number of files
in subversion.

A patch against HEAD (6415) is attached.

Log message:

* subversion/include/svn_wc.h
  (svn_wc_notify_func_t): Add an extra parameter to pass the deltas count

* subversion/clients/cmdline/feedback.c
  (notify): Display better deltas transfer progress indication

* subversion/libsvn_client/commit_util.c
  (svn_client__do_commit): Pass the number of deltas to the notification
function
  (): Patch function calls due to change in svn_wc_notify_func_t

* subversion/libsvn_wc/adm_crawler.c
* subversion/libsvn_wc/adm_ops.c
* subversion/libsvn_wc/status.c
* subversion/libsvn_wc/update_editor.c
* subversion/libsvn_client/switch.c
* subversion/libsvn_client/externals.c
* subversion/libsvn_client/diff.c
* subversion/libsvn_client/repos_diff.c
* subversion/libsvn_client/export.c
* subversion/libsvn_client/commit.c: Patch function calls due to change
in svn_wc_notify_func_t

Mukund

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by cm...@collab.net.

Mukund <mu...@tessna.com> writes:

> Hi all
> 
> The attached patch contains a better deltas transfer progress indicator for
> the 'svn' command-line client. A visual example of what it does can be found
> here: http://www.mukund.org/temp/svn.png
> 
> Due to the nature of the patch, the notification API had to be slightly
> modified (svn_wc_notify_func_t), resulting in changes to a number of files
> in subversion.
> 
> A patch against HEAD (6415) is attached.

First a comment: Your patch contains tabstops -- that's a no-no for
Subversion.

Now some questions: How does this patch behave on Windows?  In a
terminal window that doesn't gracefully support the use of \r?  When
used with a pipe into another program, or when redirected?

I like the idea behind what's being done, but I'll be out for blood if
I run this in my Emacs *shell* window and I see a full line of text
for every file content transmission of a commit.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by Philip Martin <ph...@codematters.co.uk>.

Mukund <mu...@tessna.com> writes:

> On Wed, Jul 09, 2003 at 02:26:27PM +0100, Philip Martin wrote:
> | Suppose my application makes several Subversion API calls in
> | succesion.  Am I allowed to reuse the feedback baton?  When should I
> | reset the state in the baton?  How does the reset interact with the
> | the cancel func?  Does your choice of API mean that I am unable to
> | reuse a baton?  Is that a bikeshed?
>
> I am unable to follow what you are saying, maybe due to my lack of
> knowledge of Subversion.

I was asking if I can reuse a baton

    svn_client_ctx_t ctx;
    ctx.notify_func = foo;
    ctx.notify_baton = bar;
    svn_client_update(... &ctx, ...);
    svn_client_commit(... &ctx, ...);
    svn_client_update(... &ctx, ...);

> The only field in the baton which the
> patch touches is a newly added one -- a counter. The counter is reset
> every time during the first TX, i.e., when sent_first_txdelta in the
> baton is FALSE.

Well that covers this particular callback/baton.  What if I write
another application?  Your choice of API will affect the way I can use
the callback/baton.

Philip goes and reads the patch...

I see we already have some state in the feedback baton.  If the API
passed two numbers then sent_first_txdelta could be removed from the
baton.  The received_some_change state would have to remain, but that
is a slightly different category since it does not affect the
callback's output.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by Mukund <mu...@tessna.com>.

On Wed, Jul 09, 2003 at 02:26:27PM +0100, Philip Martin wrote:
| Suppose my application makes several Subversion API calls in
| succesion.  Am I allowed to reuse the feedback baton?  When should I
| reset the state in the baton?  How does the reset interact with the
| the cancel func?  Does your choice of API mean that I am unable to
| reuse a baton?  Is that a bikeshed?

I am unable to follow what you are saying, maybe due to my lack of
knowledge of Subversion. The only field in the baton which the
patch touches is a newly added one -- a counter. The counter is reset
every time during the first TX, i.e., when sent_first_txdelta in the
baton is FALSE.

Mukund


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by Philip Martin <ph...@codematters.co.uk>.

Mukund <mu...@tessna.com> writes:

> On Wed, Jul 09, 2003 at 02:05:35PM +0100, Philip Martin wrote:
> | Since there is only one integer, the callback itself must have state
> | information to count the number of times it is called.  Is this the
> | best way to do it?  Perhaps the state should be in the calling
> | library, with two numbers being passed to the callback.
>
> The feature is quite a colourful bikeshed already :-)

I don't think API design is a bikeshed.

> I cannot find advantages or disadvantages of either approach.
> An argument would be that since all clients would need to have a counter
> to show a progress, why not implement this in the client library.
> A counter argument would be since all clients do not need to show
> progress they don't need a counter, and it needn't be maintained
> unnecessarily. Yet another colourful bikeshed.

Suppose my application makes several Subversion API calls in
succesion.  Am I allowed to reuse the feedback baton?  When should I
reset the state in the baton?  How does the reset interact with the
the cancel func?  Does your choice of API mean that I am unable to
reuse a baton?  Is that a bikeshed?

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by Mukund <mu...@tessna.com>.

On Wed, Jul 09, 2003 at 02:05:35PM +0100, Philip Martin wrote:
| Since there is only one integer, the callback itself must have state
| information to count the number of times it is called.  Is this the
| best way to do it?  Perhaps the state should be in the calling
| library, with two numbers being passed to the callback.

The feature is quite a colourful bikeshed already :-)

I cannot find advantages or disadvantages of either approach.
An argument would be that since all clients would need to have a counter
to show a progress, why not implement this in the client library.
A counter argument would be since all clients do not need to show
progress they don't need a counter, and it needn't be maintained
unnecessarily. Yet another colourful bikeshed. Submit your patch
whichever way you want it :-) Yesterday I was going to add code to
display time taken...

Mukund


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by cm...@collab.net.

"D.J. Heap" <dj...@shadyvale.net> writes:

> On kind of a side note, I was looking at the log, prompt, and notify
> callbacks and was curious why they take a bunch of parameters instead
> of a struct of relevant data?  It seems like it would simplify the API
> and perhaps make less pain for clients when changes like this occur,
> but maybe there is a good reason for it?

Oh, how many times have I thought this very thought ... how many ... ?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by Ben Collins-Sussman <su...@collab.net>.

"D.J. Heap" <dj...@shadyvale.net> writes:

> On kind of a side note, I was looking at the log, prompt, and notify
> callbacks and was curious why they take a bunch of parameters instead
> of a struct of relevant data? 

I was *just* thinking this.  Boy, I would +1 all over such a change.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by "D.J. Heap" <dj...@shadyvale.net>.

On kind of a side note, I was looking at the log, prompt, and notify 
callbacks and was curious why they take a bunch of parameters instead of 
a struct of relevant data?  It seems like it would simplify the API and 
perhaps make less pain for clients when changes like this occur, but 
maybe there is a good reason for it?

DJ



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better progress indicator for the 'svn' command-line client

Posted by Philip Martin <ph...@codematters.co.uk>.

Ben Collins-Sussman <su...@collab.net> writes:

> Mukund <mu...@tessna.com> writes:
>
>> * subversion/include/svn_wc.h
>>   (svn_wc_notify_func_t): Add an extra parameter to pass the deltas count
>>
>> Index: subversion/include/svn_wc.h
>> ===================================================================
>> --- subversion/include/svn_wc.h	(revision 6415)
>> +++ subversion/include/svn_wc.h	(working copy)
>> @@ -333,7 +333,8 @@
>>                                        const char *mime_type,
>>                                        svn_wc_notify_state_t content_state,
>>                                        svn_wc_notify_state_t prop_state,
>> -                                      svn_revnum_t revision);
>> +                                      svn_revnum_t revision,
>> +				      int txdeltas_count);
>
> You just added a new parameter to a public API.
>
> Were you planning to document it?  :-)

Since there is only one integer, the callback itself must have state
information to count the number of times it is called.  Is this the
best way to do it?  Perhaps the state should be in the calling
library, with two numbers being passed to the callback.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org