Re: [PATCH 1/1] perf:Adding --list-opts to usage string

[Namhyung Kim]

Hi Arnaldo,

On Wed, Oct 14, 2015 at 12:10 PM, Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
> Em Wed, Oct 14, 2015 at 10:29:05AM +0800, Yunlong Song escreveu:
>> On 2015/10/13 23:24, Ramkumar Ramachandra wrote:
>> > Arnaldo Carvalho de Melo wrote:
>> >> Em Thu, Oct 08, 2015 at 10:59:50AM +0530, Sriram Raghunathan escreveu:
>> >>> Minor change, adding --list-opts to usage string. So that it is
>> >>> visible to the user on running perf --help. or just perf
>> >>> from command line.
>> >>
>> >> Ramkumar, Yunlong, are you ok with this?
>
>> > Not sure I understand the motivation, but I suppose it can't hurt to
>> > show this detail?
>
>> Agree with Ramkumar, --list-opts is redundant due to the existing [OPTIONS] in
>> the perf_usage_string[].

AFAIK the --list-cmds and --list-opts options are just for completion
scripts, not for human.  I don't think we need to expose them to
normal users.

>
> I see, thinking about it now, it seems that this is because 'perf -h'
> behaves differently from other tools, i.e.:
>
> $ perf -h
>
>  usage: perf [--version] [--help] [OPTIONS] COMMAND [ARGS]
>
>  The most commonly used perf commands are:

One nit, these are not commonly used commands, but all commands. :)
Anyway it'd be better to add option descriptions here for consistency.

>    annotate        Read perf.data (created by perf record) and display annotated code
>    archive         Create archive with object files with build-ids found in perf.data file
>    bench           General framework for benchmark suites
>    buildid-cache   Manage build-id cache.
>    buildid-list    List the buildids in a perf.data file
> <SNIP>
>    test            Runs sanity tests.
>    timechart       Tool to visualize total system behavior during a workload
>    top             System profiling tool.
>    trace           strace inspired tool
>    probe           Define new dynamic tracepoints
>
>  See 'perf help COMMAND' for more information on a specific command.
>
> --------------------------
>
> While:
>
> $ perf stat -h
>
>  usage: perf stat [<options>] [<command>]
>
>     -T, --transaction     hardware transaction statistics
>     -e, --event <event>   event selector. use 'perf list' to list available events
>         --filter <filter>
>                           event filter
>     -i, --no-inherit      child tasks do not inherit counters
>     -p, --pid <pid>       stat events on existing process id
>     -t, --tid <tid>       stat events on existing thread id
>     -a, --all-cpus        system-wide collection from all CPUs
> <SNIP>
>     -I, --interval-print <n>
>                           print counts at regular interval in ms (>= 10)
>         --per-socket      aggregate counts per processor socket
>         --per-core        aggregate counts per physical processor core
>         --per-thread      aggregate counts per thread
>     -D, --delay <n>       ms to wait before starting measurement after program start
>
> --------------------------
>
> One doesn't show what options can be used, the other does, so there is
> an inconsistency, this and the fact that 'perf -h' outputs to stdout,
> 'perf stat -h' and the other builtins output to stderr. I think all
> should output to stdout, just like 'ls --help', what do you think?

I'm ok with changing to stdout.

Thanks,
Namhyung
--
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/