Re: [gdb/cli] Add a progress meter

Tom Tromey
2020-12-11T19:21:08Z

>>>>> "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.

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

I think s/notify/notifies/

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, 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.

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.

Tom