Re: [PATCH 5/5] perf timechart: add emphasize option

[Ingo Molnar]

* Stanislav Fomichev <stfomichev@xxxxxxxxxxxxxx> wrote:

> This option highlights tasks (using different color) that run more than given
> duration or tasks with given name.

Please give a sample usage command line and a sample output. (the 
highlighting can be done using ASCII escape sequences in the changelog 
as well.)

> 
> Signed-off-by: Stanislav Fomichev <stfomichev@xxxxxxxxxxxxxx>
> ---
>  tools/perf/Documentation/perf-timechart.txt |  4 ++++
>  tools/perf/builtin-timechart.c              | 21 ++++++++++++++++++++-
>  tools/perf/util/svghelper.c                 | 23 ++++++++++++++++++++---
>  tools/perf/util/svghelper.h                 |  4 +++-
>  4 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-timechart.txt b/tools/perf/Documentation/perf-timechart.txt
> index 367c1be0551c..aac589b0d377 100644
> --- a/tools/perf/Documentation/perf-timechart.txt
> +++ b/tools/perf/Documentation/perf-timechart.txt
> @@ -62,6 +62,10 @@ $ perf timechart
>  -t::
>  --topology::
>          Sort CPUs according to topology.
> +-e::
> +--emphasize::

So -e comes with an option - the syntax to describe those in manpages 
is:

  -e::
  --emphasize=<duration_nsecs|task_name>::

or so.

> +        Emphasize tasks which were running at least given number of
> +        nanoseconds or tasks with given name.

So the information in the changelog, that 'emphasize' means 'highlight 
via coloring' is lost in the documentation of the option, making it 
rather hard for users to discover and appreciate this feature...

It's also not made clear that the option can take a numeric input, and 
if the input is numeric, it is interpreted as a task name.

So please make features more approachable!

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/