On 12/11/20 8:21 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> I've taken a look, and think that this code is more in line with gdb cli
> Tom> setup, so I've reimplemented on top of this.
> 
> Tom> Here is the first patch, adding the progress meter infrastructure
> Tom> factored out from your patch.
> 
> Not sure if I ought to review a patch that I largely wrote, but I did
> find some things I think should be done differently nowadays.
> 
> Tom> +void
> Tom> +cli_ui_out::do_progress_start (const std::string &name, int should_print)
> 
> should_print should probably be a bool, throughout this patch.
> 

Done.

> Tom> +      /* Don't actually emit anything until the first call notify us
> 
> I think s/notify/notifies/
> 

Done.

> Tom> +  if (!stream->isatty ())
> Tom> +    {
> Tom> +      fprintf_unfiltered (stream, "%s...", meter.name.c_str ());
> 
> This is probably fine but I suppose the meaning of "should_print" ought
> to be described somewhere,

Done.

> since otherwise this could seem like a bug --
> it is printing something even when should_print==false.
> 
> Tom> +  m_meters.push_back (meter);
> 
> std::move(meter) would be more efficient here, since it would avoid
> another string copy.
> 

Done.

> Tom> +    progress_meter (struct ui_out *uiout, const std::string &name,
> Tom> +		    int should_print)
> Tom> +      : m_uiout (uiout)
> 
> Here's probably where should_print should be documented.
> 
> Tom> +get_chars_per_line (void)
> 
> Just "()", not "(void)", now.
> 
> Tom> +++ b/gdb/utils.h
> Tom> @@ -364,6 +364,10 @@ extern void wrap_here (const char *);
>  
> Tom>  extern void reinitialize_more_filter (void);
>  
> Tom> +/* Return the number of characters in a line.  */
> Tom> +
> Tom> +extern int get_chars_per_line (void);
> 
> Here too.

Done.

I've also fixed the first-progress-call-has-howmuch=1.0 problem
mentioned in the original review conversation with Pedro, by adding a
new enum meter_state element PROGRESS.

I've also fixed the problem that when f.i. run with -batch,
chars_per_line is zero. The previous version of the patch was printing
"[]" in that case.

Thanks,
- Tom