Re: progress API

[Tim Janik <timj gtk org>]

On 29 Mar 2000, Havoc Pennington wrote:

> 
> Hi,
> 
> GtkProgress/GtkProgressBar have serious problems.
> 
>  - Only 3 or 4 functions are really needed for 95% of progress  
>    interfaces; GtkProgress[Bar] has about 25 functions, and 
>    doesn't even include these 3 or 4. This means a steep 
>    learning curve.
>  - In activity mode, the API involves setting the adjustment 
>    to any random value, just to have the side effect of 
>    calling the progress bar update function - the adjustment
>    is totally ignored in activity mode
>  - You set the activity step as a pixel value, which means to 
>    set the activity step you basically need to connect to 
>    size_allocate
>  - There are ctree_set_expander_style() functions, to randomly 
>    change look-and-feel for no good reason
>  - The split between GtkProgress and GtkProgressBar makes no sense 
>    to me whatsoever.
>  
> This is a big wart on GTK and makes people waste lots of time, both
> learning and using the interface.
> 
> So, I have added what I feel is the correct API, and marked all the
> rest deprecated. However, the changes are 100% backward-compatible and
> should break no existing code.
> 
> New API:
> 
> void       gtk_progress_bar_pulse                (GtkProgressBar *pbar);
> void       gtk_progress_bar_set_fraction         (GtkProgressBar *pbar,
>                                                   gfloat          fraction);
> void       gtk_progress_bar_set_text             (GtkProgressBar *pbar,
>                                                   const gchar    *text);
> void       gtk_progress_bar_set_pulse_frequency  (GtkProgressBar *pbar,
>                                                   gfloat          fraction);
> void       gtk_progress_bar_set_orientation      (GtkProgressBar *pbar,
> 						  GtkProgressBarOrientation orientation);
> 

put this rationale into the header file please,
preferable as a block comment, preceeding all five function definitions.
comments, btw, in gtk code are of the following styles:

/* one line comment */

/* multi line
 * comment, set out with
 * preceeding asterisks
 */

/* occasional use of one liner to seperate stuff
 */

(the last one is pretty rare actually)


> Rationale:
> 
>  - pulse() means "I have made progress but don't know how much."
>    Activity mode is entered, and if we're already in activity mode
>    the little block moves.
>  - set_fraction() means "I have between 0.0 and 1.0 of the task
>    finished" and automatically leaves activity mode.
>  - set_pulse_frequency() controls the fraction of distance each 
>    pulse() moves the activity block; useful to tune the speed of 
>    your progress bar
>  - set_text() sets any text you want, NOT a format string. 
>    If you want a format, just do g_strdup_printf () or something,
>    then set the text. This way people don't have to worry about 
>    escaping strings, and they can do custom formatting easily.
>  - set_orientation() seems pretty questionable to me (since a vertical
>    bar is probably not a progress bar, it's probably a range indicator
>    of some kind), but is a "why not" sort of thing. I'm willing 
>    to take this out.

> 
> To know why this patch should go in GTK, just look at the current
> GtkProgress/GtkProgressBar docs, and then imagine the same docs
> fitting on a single, easy-to-understand page with this new API. Then
> try to think of a progress-indication case where this new API is
> insufficient or even particularly inconvenient. Can we say "gratuitous
> complexity"?

ok, haven't had a look at the docs, but had to figure behaviour for
BEAST some while ago, and it took me almost a day to get behaviour
(pulse and precentage) right.
the API you propose looks very good, just one thing on the implementation:

> +void
> +gtk_progress_bar_pulse (GtkProgressBar *pbar)
> +{  
> +  g_return_if_fail (pbar != NULL);
> +  g_return_if_fail (GTK_IS_PROGRESS_BAR (pbar));
> +
> +  /* If we don't know the percentage, we must want activity mode. */
> +  gtk_progress_set_activity_mode (GTK_PROGRESS (pbar), TRUE);
> +
> +  /* Sigh. */
> +  gtk_progress_bar_real_update (GTK_PROGRESS (pbar));
> +}

not only "Sigh." ;)
having to call gtk_progress_bar_real_update() directly sucks majorly,
instead, what about making GtkProgress::update a real signal, and have
void gtk_progress_update (GtkProgress *progress);
emitting it?
yes, that breaks with the arguments of gtk_progress_bar_update(), but then
you deprecated that anyways. also, having a signal here is fairly important
if you want people to be able to easily automate the
g_strdup_printf()/gtk_progress_bar_set_text() task.

> 
> Patch appended.

thanks, still have to get back on GtkDialog though ,)

> 
> Havoc
> 

---
ciaoTJ