[PATCH 0/2] perf tools: Basic bash completion support

[PATCH 0/2] perf tools: Basic bash completion support

[Frederic Weisbecker]

Hey,

Basic bash completion support. Only support perf subcommands and most -e basic
event descriptor (no grouping).

I just have a small issue with tracepoints because of their ":" in the middle.
It auto completes as long as we haven't yet reached the semicolon. Otherwise
we need to add a double quote in the beginning of the expression. I'm quite
a newbie in bash completion though, so I might find a subtelty later to solve
this.

Frederic Weisbecker (2):
  perf tools: Initial bash completion support
  perf tools: Support for events bash completion

 tools/perf/Makefile            |    1 +
 tools/perf/bash_completion     |   24 ++++++++++++++
 tools/perf/builtin-list.c      |   14 ++++---
 tools/perf/perf.c              |   69 ++++++++++++++++++++++-----------------
 tools/perf/util/parse-events.c |   70 +++++++++++++++++++++++++---------------
 tools/perf/util/parse-events.h |    7 ++--
 6 files changed, 120 insertions(+), 65 deletions(-)
 create mode 100644 tools/perf/bash_completion

-- 
1.7.5.4

[PATCH 1/2] perf tools: Initial bash completion support

[Frederic Weisbecker]

This implements bash completion for perf subcommands such
as record, report, script, probe, etc...

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/Makefile        |    1 +
 tools/perf/bash_completion |   20 +++++++++++++
 tools/perf/perf.c          |   69 +++++++++++++++++++++++++-------------------
 3 files changed, 60 insertions(+), 30 deletions(-)
 create mode 100644 tools/perf/bash_completion

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 35655c3..4000d72 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -951,6 +951,7 @@ install: all
 	$(INSTALL) scripts/python/Perf-Trace-Util/lib/Perf/Trace/* -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python/Perf-Trace-Util/lib/Perf/Trace'
 	$(INSTALL) scripts/python/*.py -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python'
 	$(INSTALL) scripts/python/bin/* -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python/bin'
+	$(INSTALL) -m 755 bash_completion /etc/bash_completion.d/perf
 
 install-python_ext:
 	$(PYTHON_WORD) util/setup.py --quiet install --root='/$(DESTDIR_SQ)'
diff --git a/tools/perf/bash_completion b/tools/perf/bash_completion
new file mode 100644
index 0000000..3547703
--- /dev/null
+++ b/tools/perf/bash_completion
@@ -0,0 +1,20 @@
+# perf completion
+
+have perf &&
+_perf()
+{
+	local cur
+
+	COMPREPLY=()
+	_get_comp_words_by_ref cur
+
+	# List perf subcommands
+	if [ $COMP_CWORD -eq 1 ]; then
+		cmds=$(perf --list-cmds)
+		COMPREPLY=( $( compgen -W '$cmds' -- "$cur" ) )
+	# Fall down to list regular files
+	else
+		_filedir
+	fi
+} &&
+complete -F _perf perf
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 2b2e225..db37ee3 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -24,6 +24,37 @@ const char perf_more_info_string[] =
 int use_browser = -1;
 static int use_pager = -1;
 
+struct cmd_struct {
+	const char *cmd;
+	int (*fn)(int, const char **, const char *);
+	int option;
+};
+
+static struct cmd_struct commands[] = {
+	{ "buildid-cache", cmd_buildid_cache, 0 },
+	{ "buildid-list", cmd_buildid_list, 0 },
+	{ "diff",	cmd_diff,	0 },
+	{ "evlist",	cmd_evlist,	0 },
+	{ "help",	cmd_help,	0 },
+	{ "list",	cmd_list,	0 },
+	{ "record",	cmd_record,	0 },
+	{ "report",	cmd_report,	0 },
+	{ "bench",	cmd_bench,	0 },
+	{ "stat",	cmd_stat,	0 },
+	{ "timechart",	cmd_timechart,	0 },
+	{ "top",	cmd_top,	0 },
+	{ "annotate",	cmd_annotate,	0 },
+	{ "version",	cmd_version,	0 },
+	{ "script",	cmd_script,	0 },
+	{ "sched",	cmd_sched,	0 },
+	{ "probe",	cmd_probe,	0 },
+	{ "kmem",	cmd_kmem,	0 },
+	{ "lock",	cmd_lock,	0 },
+	{ "kvm",	cmd_kvm,	0 },
+	{ "test",	cmd_test,	0 },
+	{ "inject",	cmd_inject,	0 },
+};
+
 struct pager_config {
 	const char *cmd;
 	int val;
@@ -160,6 +191,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			fprintf(stderr, "dir: %s\n", debugfs_mountpoint);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--list-cmds")) {
+			unsigned int i;
+
+			for (i = 0; i < ARRAY_SIZE(commands); i++) {
+				struct cmd_struct *p = commands+i;
+				printf("%s ", p->cmd);
+			}
+			exit(0);
 		} else {
 			fprintf(stderr, "Unknown option: %s\n", cmd);
 			usage(perf_usage_string);
@@ -245,12 +284,6 @@ const char perf_version_string[] = PERF_VERSION;
  */
 #define NEED_WORK_TREE	(1<<2)
 
-struct cmd_struct {
-	const char *cmd;
-	int (*fn)(int, const char **, const char *);
-	int option;
-};
-
 static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 {
 	int status;
@@ -296,30 +329,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 static void handle_internal_command(int argc, const char **argv)
 {
 	const char *cmd = argv[0];
-	static struct cmd_struct commands[] = {
-		{ "buildid-cache", cmd_buildid_cache, 0 },
-		{ "buildid-list", cmd_buildid_list, 0 },
-		{ "diff",	cmd_diff,	0 },
-		{ "evlist",	cmd_evlist,	0 },
-		{ "help",	cmd_help,	0 },
-		{ "list",	cmd_list,	0 },
-		{ "record",	cmd_record,	0 },
-		{ "report",	cmd_report,	0 },
-		{ "bench",	cmd_bench,	0 },
-		{ "stat",	cmd_stat,	0 },
-		{ "timechart",	cmd_timechart,	0 },
-		{ "top",	cmd_top,	0 },
-		{ "annotate",	cmd_annotate,	0 },
-		{ "version",	cmd_version,	0 },
-		{ "script",	cmd_script,	0 },
-		{ "sched",	cmd_sched,	0 },
-		{ "probe",	cmd_probe,	0 },
-		{ "kmem",	cmd_kmem,	0 },
-		{ "lock",	cmd_lock,	0 },
-		{ "kvm",	cmd_kvm,	0 },
-		{ "test",	cmd_test,	0 },
-		{ "inject",	cmd_inject,	0 },
-	};
 	unsigned int i;
 	static const char ext[] = STRIP_EXTENSION;
 
-- 
1.7.5.4

[PATCH 2/2] perf tools: Support for events bash completion

[Frederic Weisbecker]

Add basic bash completion for the -e option in record, top
and stat subcommands. Only hardware, software and tracepoint
events are supported.

Breakpoints, raw events and events grouping completion
need more thinking.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/bash_completion     |    6 +++-
 tools/perf/builtin-list.c      |   14 ++++---
 tools/perf/util/parse-events.c |   70 +++++++++++++++++++++++++---------------
 tools/perf/util/parse-events.h |    7 ++--
 4 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/tools/perf/bash_completion b/tools/perf/bash_completion
index 3547703..25f4d99 100644
--- a/tools/perf/bash_completion
+++ b/tools/perf/bash_completion
@@ -6,12 +6,16 @@ _perf()
 	local cur
 
 	COMPREPLY=()
-	_get_comp_words_by_ref cur
+	_get_comp_words_by_ref cur prev
 
 	# List perf subcommands
 	if [ $COMP_CWORD -eq 1 ]; then
 		cmds=$(perf --list-cmds)
 		COMPREPLY=( $( compgen -W '$cmds' -- "$cur" ) )
+	# List possible events for -e option
+	elif [[ $prev == "-e" && "${COMP_WORDS[1]}" == @(record|stat|top) ]]; then
+		cmds=$(perf list --raw-dump)
+		COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )
 	# Fall down to list regular files
 	else
 		_filedir
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 6313b6e..bdcff81 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,15 +19,15 @@ int cmd_list(int argc, const char **argv, const char *prefix __used)
 	setup_pager();
 
 	if (argc == 1)
-		print_events(NULL);
+		print_events(NULL, false);
 	else {
 		int i;
 
 		for (i = 1; i < argc; ++i) {
-			if (i > 1)
+			if (i > 2)
 				putchar('\n');
 			if (strncmp(argv[i], "tracepoint", 10) == 0)
-				print_tracepoint_events(NULL, NULL);
+				print_tracepoint_events(NULL, NULL, false);
 			else if (strcmp(argv[i], "hw") == 0 ||
 				 strcmp(argv[i], "hardware") == 0)
 				print_events_type(PERF_TYPE_HARDWARE);
@@ -36,13 +36,15 @@ int cmd_list(int argc, const char **argv, const char *prefix __used)
 				print_events_type(PERF_TYPE_SOFTWARE);
 			else if (strcmp(argv[i], "cache") == 0 ||
 				 strcmp(argv[i], "hwcache") == 0)
-				print_hwcache_events(NULL);
+				print_hwcache_events(NULL, false);
+			else if (strcmp(argv[i], "--raw-dump") == 0)
+				print_events(NULL, true);
 			else {
 				char *sep = strchr(argv[i], ':'), *s;
 				int sep_idx;
 
 				if (sep == NULL) {
-					print_events(argv[i]);
+					print_events(argv[i], false);
 					continue;
 				}
 				sep_idx = sep - argv[i];
@@ -51,7 +53,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __used)
 					return -1;
 
 				s[sep_idx] = '\0';
-				print_tracepoint_events(s, s + sep_idx + 1);
+				print_tracepoint_events(s, s + sep_idx + 1, false);
 				free(s);
 			}
 		}
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 74a5af4..30dba72 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -799,7 +799,8 @@ static const char * const event_type_descriptors[] = {
  * Print the events from <debugfs_mount_point>/tracing/events
  */
 
-void print_tracepoint_events(const char *subsys_glob, const char *event_glob)
+void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+			     bool name_only)
 {
 	DIR *sys_dir, *evt_dir;
 	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
@@ -829,6 +830,11 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob)
 			    !strglobmatch(evt_dirent.d_name, event_glob))
 				continue;
 
+			if (name_only) {
+				printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);
+				continue;
+			}
+
 			snprintf(evt_path, MAXPATHLEN, "%s:%s",
 				 sys_dirent.d_name, evt_dirent.d_name);
 			printf("  %-50s [%s]\n", evt_path,
@@ -906,7 +912,7 @@ void print_events_type(u8 type)
 		__print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
 }
 
-int print_hwcache_events(const char *event_glob)
+int print_hwcache_events(const char *event_glob, bool name_only)
 {
 	unsigned int type, op, i, printed = 0;
 	char name[64];
@@ -923,8 +929,11 @@ int print_hwcache_events(const char *event_glob)
 				if (event_glob != NULL && !strglobmatch(name, event_glob))
 					continue;
 
-				printf("  %-50s [%s]\n", name,
-					event_type_descriptors[PERF_TYPE_HW_CACHE]);
+				if (name_only)
+					printf("%s ", name);
+				else
+					printf("  %-50s [%s]\n", name,
+					       event_type_descriptors[PERF_TYPE_HW_CACHE]);
 				++printed;
 			}
 		}
@@ -934,7 +943,8 @@ int print_hwcache_events(const char *event_glob)
 }
 
 static void print_symbol_events(const char *event_glob, unsigned type,
-				struct event_symbol *syms, unsigned max)
+				struct event_symbol *syms, unsigned max,
+				bool name_only)
 {
 	unsigned i, printed = 0;
 	char name[MAX_NAME_LEN];
@@ -946,6 +956,11 @@ static void print_symbol_events(const char *event_glob, unsigned type,
 		      (syms->alias && strglobmatch(syms->alias, event_glob))))
 			continue;
 
+		if (name_only) {
+			printf("%s ", syms->symbol);
+			continue;
+		}
+
 		if (strlen(syms->alias))
 			snprintf(name, MAX_NAME_LEN, "%s OR %s", syms->symbol, syms->alias);
 		else
@@ -963,39 +978,42 @@ static void print_symbol_events(const char *event_glob, unsigned type,
 /*
  * Print the help text for the event symbols:
  */
-void print_events(const char *event_glob)
+void print_events(const char *event_glob, bool name_only)
 {
-
-	printf("\n");
-	printf("List of pre-defined events (to be used in -e):\n");
+	if (!name_only) {
+		printf("\n");
+		printf("List of pre-defined events (to be used in -e):\n");
+	}
 
 	print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
-			    event_symbols_hw, PERF_COUNT_HW_MAX);
+			    event_symbols_hw, PERF_COUNT_HW_MAX, name_only);
 
 	print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
-			    event_symbols_sw, PERF_COUNT_SW_MAX);
+			    event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
 
-	print_hwcache_events(event_glob);
+	print_hwcache_events(event_glob, name_only);
 
 	if (event_glob != NULL)
 		return;
 
-	printf("\n");
-	printf("  %-50s [%s]\n",
-	       "rNNN",
-	       event_type_descriptors[PERF_TYPE_RAW]);
-	printf("  %-50s [%s]\n",
-	       "cpu/t1=v1[,t2=v2,t3 ...]/modifier",
-	       event_type_descriptors[PERF_TYPE_RAW]);
-	printf("   (see 'perf list --help' on how to encode it)\n");
-	printf("\n");
-
-	printf("  %-50s [%s]\n",
-			"mem:<addr>[:access]",
+	if (!name_only) {
+		printf("\n");
+		printf("  %-50s [%s]\n",
+		       "rNNN",
+		       event_type_descriptors[PERF_TYPE_RAW]);
+		printf("  %-50s [%s]\n",
+		       "cpu/t1=v1[,t2=v2,t3 ...]/modifier",
+		       event_type_descriptors[PERF_TYPE_RAW]);
+		printf("   (see 'perf list --help' on how to encode it)\n");
+		printf("\n");
+
+		printf("  %-50s [%s]\n",
+		       "mem:<addr>[:access]",
 			event_type_descriptors[PERF_TYPE_BREAKPOINT]);
-	printf("\n");
+		printf("\n");
+	}
 
-	print_tracepoint_events(NULL, NULL);
+	print_tracepoint_events(NULL, NULL, name_only);
 }
 
 int parse_events__is_hardcoded_term(struct parse_events__term *term)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ee9c218..6fd9307 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -96,10 +96,11 @@ void parse_events_update_lists(struct list_head *list_event,
 void parse_events_error(void *data, void *scanner, char const *msg);
 int parse_events__test(void);
 
-void print_events(const char *event_glob);
+void print_events(const char *event_glob, bool name_only);
 void print_events_type(u8 type);
-void print_tracepoint_events(const char *subsys_glob, const char *event_glob);
-int print_hwcache_events(const char *event_glob);
+void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+			     bool name_only);
+int print_hwcache_events(const char *event_glob, bool name_only);
 extern int is_valid_tracepoint(const char *event_string);
 
 extern int valid_debugfs_mount(const char *debugfs);
-- 
1.7.5.4

Re: [PATCH 0/2] perf tools: Basic bash completion support

[Frederic Weisbecker]

On Tue, Aug 07, 2012 at 03:19:44PM +0200, Frederic Weisbecker wrote:
> Hey,
> 
> Basic bash completion support. Only support perf subcommands and most -e basic
> event descriptor (no grouping).
> 
> I just have a small issue with tracepoints because of their ":" in the middle.
> It auto completes as long as we haven't yet reached the semicolon. Otherwise
> we need to add a double quote in the beginning of the expression. I'm quite
> a newbie in bash completion though, so I might find a subtelty later to solve
> this.

Tips: for testing, you need to "make install" and update the bash completion
scripts:

	# make install
	$ . /etc/bash_completion

Re: [PATCH 1/2] perf tools: Initial bash completion support

[David Ahern]

On 8/7/12 7:19 AM, Frederic Weisbecker wrote:
> This implements bash completion for perf subcommands such
> as record, report, script, probe, etc...

Love it!

>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@gmail.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Stephane Eranian <eranian@google.com>
> ---
>   tools/perf/Makefile        |    1 +
>   tools/perf/bash_completion |   20 +++++++++++++
>   tools/perf/perf.c          |   69 +++++++++++++++++++++++++-------------------
>   3 files changed, 60 insertions(+), 30 deletions(-)
>   create mode 100644 tools/perf/bash_completion
>
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 35655c3..4000d72 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -951,6 +951,7 @@ install: all
>   	$(INSTALL) scripts/python/Perf-Trace-Util/lib/Perf/Trace/* -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python/Perf-Trace-Util/lib/Perf/Trace'
>   	$(INSTALL) scripts/python/*.py -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python'
>   	$(INSTALL) scripts/python/bin/* -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python/bin'
> +	$(INSTALL) -m 755 bash_completion /etc/bash_completion.d/perf

$(DESTDIR_SQ) is need in front of the destination.

David

Re: [PATCH 0/2] perf tools: Basic bash completion support

[David Ahern]

On 8/7/12 7:22 AM, Frederic Weisbecker wrote:
> On Tue, Aug 07, 2012 at 03:19:44PM +0200, Frederic Weisbecker wrote:
>> Hey,
>>
>> Basic bash completion support. Only support perf subcommands and most -e basic
>> event descriptor (no grouping).
>>
>> I just have a small issue with tracepoints because of their ":" in the middle.
>> It auto completes as long as we haven't yet reached the semicolon. Otherwise
>> we need to add a double quote in the beginning of the expression. I'm quite
>> a newbie in bash completion though, so I might find a subtelty later to solve
>> this.
>
> Tips: for testing, you need to "make install" and update the bash completion
> scripts:
>
> 	# make install
> 	$ . /etc/bash_completion
>

ANd you need to make sure the PATH hits the updated binary and not the 
default other wise you end up with:

/tmp/pbuild/perf recUnknown option: --list-cmds

  Usage: perf [--version] [--help] COMMAND [ARGS]
Unknown option: --list-cmds

It's calling /usr/bin/perf with --list-cmds, versus the perf command I 
am running (/tmp/pbuild/perf). Any way to teach the completion to use 
the perf binary that the user is running?

David

Re: [PATCH 2/2] perf tools: Support for events bash completion

[David Ahern]

On 8/7/12 7:19 AM, Frederic Weisbecker wrote:
> Add basic bash completion for the -e option in record, top
> and stat subcommands. Only hardware, software and tracepoint
> events are supported.
>
> Breakpoints, raw events and events grouping completion
> need more thinking.
>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@gmail.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Stephane Eranian <eranian@google.com>
> ---
>   tools/perf/bash_completion     |    6 +++-
>   tools/perf/builtin-list.c      |   14 ++++---
>   tools/perf/util/parse-events.c |   70 +++++++++++++++++++++++++---------------
>   tools/perf/util/parse-events.h |    7 ++--
>   4 files changed, 61 insertions(+), 36 deletions(-)
>
> diff --git a/tools/perf/bash_completion b/tools/perf/bash_completion
> index 3547703..25f4d99 100644
> --- a/tools/perf/bash_completion
> +++ b/tools/perf/bash_completion
> @@ -6,12 +6,16 @@ _perf()
>   	local cur
>
>   	COMPREPLY=()
> -	_get_comp_words_by_ref cur
> +	_get_comp_words_by_ref cur prev
>
>   	# List perf subcommands
>   	if [ $COMP_CWORD -eq 1 ]; then
>   		cmds=$(perf --list-cmds)
>   		COMPREPLY=( $( compgen -W '$cmds' -- "$cur" ) )
> +	# List possible events for -e option
> +	elif [[ $prev == "-e" && "${COMP_WORDS[1]}" == @(record|stat|top) ]]; then
> +		cmds=$(perf list --raw-dump)
> +		COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )
>   	# Fall down to list regular files
>   	else
>   		_filedir

Any reason to show a file list except for -i and -o options? e.g.,

diff --git a/tools/perf/bash_completion b/tools/perf/bash_completion
index 25f4d99..be97349 100644
--- a/tools/perf/bash_completion
+++ b/tools/perf/bash_completion
@@ -17,7 +17,7 @@ _perf()
         cmds=$(perf list --raw-dump)
         COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )
     # Fall down to list regular files
-   else
+   elif [[ $prev == "-o" || $prev == "-i" ]]; then
         _filedir
     fi
  } &&

Re: [PATCH 0/2] perf tools: Basic bash completion support

[Frederic Weisbecker]

On Tue, Aug 07, 2012 at 08:18:12AM -0600, David Ahern wrote:
> On 8/7/12 7:22 AM, Frederic Weisbecker wrote:
> >On Tue, Aug 07, 2012 at 03:19:44PM +0200, Frederic Weisbecker wrote:
> >>Hey,
> >>
> >>Basic bash completion support. Only support perf subcommands and most -e basic
> >>event descriptor (no grouping).
> >>
> >>I just have a small issue with tracepoints because of their ":" in the middle.
> >>It auto completes as long as we haven't yet reached the semicolon. Otherwise
> >>we need to add a double quote in the beginning of the expression. I'm quite
> >>a newbie in bash completion though, so I might find a subtelty later to solve
> >>this.
> >
> >Tips: for testing, you need to "make install" and update the bash completion
> >scripts:
> >
> >	# make install
> >	$ . /etc/bash_completion
> >
> 
> ANd you need to make sure the PATH hits the updated binary and not
> the default other wise you end up with:
> 
> /tmp/pbuild/perf recUnknown option: --list-cmds
> 
>  Usage: perf [--version] [--help] COMMAND [ARGS]
> Unknown option: --list-cmds
> 
> It's calling /usr/bin/perf with --list-cmds, versus the perf command
> I am running (/tmp/pbuild/perf). Any way to teach the completion to
> use the perf binary that the user is running?

Ah good point.

Does the below work for you? I'll respin with that change.

diff --git a/tools/perf/bash_completion b/tools/perf/bash_completion
index 25f4d99..cba72a9 100644
--- a/tools/perf/bash_completion
+++ b/tools/perf/bash_completion
@@ -3,18 +3,20 @@
 have perf &&
 _perf()
 {
-	local cur
+	local cur cmd
 
 	COMPREPLY=()
 	_get_comp_words_by_ref cur prev
 
+	cmd=${COMP_WORDS[0]}
+
 	# List perf subcommands
 	if [ $COMP_CWORD -eq 1 ]; then
-		cmds=$(perf --list-cmds)
+		cmds=$($cmd --list-cmds)
 		COMPREPLY=( $( compgen -W '$cmds' -- "$cur" ) )
 	# List possible events for -e option
 	elif [[ $prev == "-e" && "${COMP_WORDS[1]}" == @(record|stat|top) ]]; then
-		cmds=$(perf list --raw-dump)
+		cmds=$($cmd list --raw-dump)
 		COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )
 	# Fall down to list regular files
 	else

Re: [PATCH 2/2] perf tools: Support for events bash completion

[Frederic Weisbecker]

On Tue, Aug 07, 2012 at 08:48:04AM -0600, David Ahern wrote:
> On 8/7/12 7:19 AM, Frederic Weisbecker wrote:
> >Add basic bash completion for the -e option in record, top
> >and stat subcommands. Only hardware, software and tracepoint
> >events are supported.
> >
> >Breakpoints, raw events and events grouping completion
> >need more thinking.
> >
> >Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> >Cc: David Ahern <dsahern@gmail.com>
> >Cc: Ingo Molnar <mingo@kernel.org>
> >Cc: Jiri Olsa <jolsa@redhat.com>
> >Cc: Namhyung Kim <namhyung@gmail.com>
> >Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> >Cc: Stephane Eranian <eranian@google.com>
> >---
> >  tools/perf/bash_completion     |    6 +++-
> >  tools/perf/builtin-list.c      |   14 ++++---
> >  tools/perf/util/parse-events.c |   70 +++++++++++++++++++++++++---------------
> >  tools/perf/util/parse-events.h |    7 ++--
> >  4 files changed, 61 insertions(+), 36 deletions(-)
> >
> >diff --git a/tools/perf/bash_completion b/tools/perf/bash_completion
> >index 3547703..25f4d99 100644
> >--- a/tools/perf/bash_completion
> >+++ b/tools/perf/bash_completion
> >@@ -6,12 +6,16 @@ _perf()
> >  	local cur
> >
> >  	COMPREPLY=()
> >-	_get_comp_words_by_ref cur
> >+	_get_comp_words_by_ref cur prev
> >
> >  	# List perf subcommands
> >  	if [ $COMP_CWORD -eq 1 ]; then
> >  		cmds=$(perf --list-cmds)
> >  		COMPREPLY=( $( compgen -W '$cmds' -- "$cur" ) )
> >+	# List possible events for -e option
> >+	elif [[ $prev == "-e" && "${COMP_WORDS[1]}" == @(record|stat|top) ]]; then
> >+		cmds=$(perf list --raw-dump)
> >+		COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )
> >  	# Fall down to list regular files
> >  	else
> >  		_filedir
> 
> Any reason to show a file list except for -i and -o options? e.g.,

Yeah, for example with perf record when you pass a command to launch and profile.

In any case I think it's a better idea to keep this as a default. Not breaking the
pre-existing default completion in the guarantee that the new completion is going
to be more useful than a burden.

> 
> diff --git a/tools/perf/bash_completion b/tools/perf/bash_completion
> index 25f4d99..be97349 100644
> --- a/tools/perf/bash_completion
> +++ b/tools/perf/bash_completion
> @@ -17,7 +17,7 @@ _perf()
>         cmds=$(perf list --raw-dump)
>         COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )
>     # Fall down to list regular files
> -   else
> +   elif [[ $prev == "-o" || $prev == "-i" ]]; then
>         _filedir
>     fi
>  } &&
> 

Re: [PATCH 1/2] perf tools: Initial bash completion support

[Frederic Weisbecker]

On Tue, Aug 07, 2012 at 08:11:46AM -0600, David Ahern wrote:
> On 8/7/12 7:19 AM, Frederic Weisbecker wrote:
> >This implements bash completion for perf subcommands such
> >as record, report, script, probe, etc...
> 
> Love it!
> 
> >
> >Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> >Cc: David Ahern <dsahern@gmail.com>
> >Cc: Ingo Molnar <mingo@kernel.org>
> >Cc: Jiri Olsa <jolsa@redhat.com>
> >Cc: Namhyung Kim <namhyung@gmail.com>
> >Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> >Cc: Stephane Eranian <eranian@google.com>
> >---
> >  tools/perf/Makefile        |    1 +
> >  tools/perf/bash_completion |   20 +++++++++++++
> >  tools/perf/perf.c          |   69 +++++++++++++++++++++++++-------------------
> >  3 files changed, 60 insertions(+), 30 deletions(-)
> >  create mode 100644 tools/perf/bash_completion
> >
> >diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> >index 35655c3..4000d72 100644
> >--- a/tools/perf/Makefile
> >+++ b/tools/perf/Makefile
> >@@ -951,6 +951,7 @@ install: all
> >  	$(INSTALL) scripts/python/Perf-Trace-Util/lib/Perf/Trace/* -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python/Perf-Trace-Util/lib/Perf/Trace'
> >  	$(INSTALL) scripts/python/*.py -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python'
> >  	$(INSTALL) scripts/python/bin/* -t '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/scripts/python/bin'
> >+	$(INSTALL) -m 755 bash_completion /etc/bash_completion.d/perf
> 
> $(DESTDIR_SQ) is need in front of the destination.

Right. Fixing this.

Thanks.

> 
> David

Re: [PATCH 0/2] perf tools: Basic bash completion support

[David Ahern]

On 8/7/12 9:45 AM, Frederic Weisbecker wrote:

> Does the below work for you? I'll respin with that change.

It does.

David

Re: [PATCH 2/2] perf tools: Support for events bash completion

[Alan Cox]

> >   		COMPREPLY=( $( compgen -W '$cmds' -- "$cur" ) )
> > +	# List possible events for -e option
> > +	elif [[ $prev == "-e" && "${COMP_WORDS[1]}" == @(record|stat|top) ]]; then
> > +		cmds=$(perf list --raw-dump)
> > +		COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )


Surely $cur should be quoted here...

Alan

Re: [PATCH 2/2] perf tools: Support for events bash completion

[Frederic Weisbecker]

On Tue, Aug 07, 2012 at 05:05:04PM +0100, Alan Cox wrote:
> > >   		COMPREPLY=( $( compgen -W '$cmds' -- "$cur" ) )
> > > +	# List possible events for -e option
> > > +	elif [[ $prev == "-e" && "${COMP_WORDS[1]}" == @(record|stat|top) ]]; then
> > > +		cmds=$(perf list --raw-dump)
> > > +		COMPREPLY=( $( compgen -W '$cmds' -- $cur ) )
> 
> 
> Surely $cur should be quoted here...

Right, fixing that too.

thanks.
 
> Alan