linux-kernel - Re: [PATCH v2 1/2] perf hists browser: Support horizontal scrolling with '<' and '>' key

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150809103542.GA3415@danjae.kornet>
Date:	Sun, 9 Aug 2015 19:35:42 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	LKML <linux-kernel@...r.kernel.org>,
	David Ahern <dsahern@...il.com>,
	Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH v2 1/2] perf hists browser: Support horizontal scrolling
 with '<' and '>' key

Hi Jiri,

On Sun, Aug 09, 2015 at 11:30:24AM +0200, Jiri Olsa wrote:
> On Sun, Aug 09, 2015 at 05:21:01PM +0900, Namhyung Kim wrote:
> > Currently perf TUI report browser doesn't support horizontal scrolling.
> > So if terminal width is smaller than the actual contents, there's no way
> > to see them.  This patch adds support horizontal movement by '<' and '>'
> > keys.
> 
> nice, I wonder we could also have some way to scroll
> by the column width.. it might be more eye friendly?
> would need to try first ;-)

Good suggesion.  Please see below..

> 
> I also tried it with SKIP_COLS_STEP=1, and it wasn't bad

OK.

> 
> how about having several scroll step options? like:
>      , . - SKIP_COLS_STEP=1
>      < > - SKIP_COLS_STEP=10
> CTRL-< > - SKIP_COLS_STEP=columns width

I tried to use CTRL but it seems not working.


> 
> we could also bind some of this to regular arrows
> with SHIFT or CTRL, bacause it's probably the most
> convenient binding for this

Yes, I agree with you.  But I don't know how to bind the arrow keys
with SHIFT or CTRL to do the thing.  So I just changed that < > to
make SKIP_COLS_STEP = column width.


Thanks,
Namhyung



>From 8e1f0a8be36895f9f37df133dcc8020e123b76e9 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@...nel.org>
Date: Sun, 9 Aug 2015 19:25:32 +0900
Subject: [PATCH] perf hists browser: Move to prev/next column start by '<' or
 '>' keys

Jiri said that it'd be more eye-friendly if it can move by column
widths.  So change the keys to do it rather than jumping 10 characters.
Also add ',' and '.' keys which reside same position in the keyboard to
move by 1 characters.

Requested-by: Jiri Olsa <jolsa@...hat.com>
Signed-off-by: Namhyung Kim <namhyung@...nel.org>
---
 tools/perf/ui/browsers/hists.c | 53 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 10af42f5283e..d62815965b50 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -20,7 +20,7 @@
 #include "map.h"
 #include "annotate.h"
 
-#define SKIP_COLS_STEP  10
+#define SKIP_COLS_STEP  1
 
 struct hist_browser {
 	struct ui_browser   b;
@@ -427,6 +427,45 @@ static void ui_browser__warn_lost_events(struct ui_browser *browser)
 		"Or reduce the sampling frequency.");
 }
 
+static void hist_browser__prev_skip_cols(struct hist_browser *browser)
+{
+	int cols = 0;
+	int curr = 0;
+	struct perf_hpp_fmt *fmt;
+	struct perf_evsel *evsel = hists_to_evsel(browser->hists);
+
+	perf_hpp__for_each_format(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
+		curr = fmt->width(fmt, NULL, evsel);
+		if (cols + curr + 2 >= browser->skip_cols) {
+			browser->skip_cols = cols;
+			break;
+		}
+
+		cols += curr + 2;
+	}
+}
+
+static void hist_browser__next_skip_cols(struct hist_browser *browser)
+{
+	int cols = 0;
+	struct perf_hpp_fmt *fmt;
+	struct perf_evsel *evsel = hists_to_evsel(browser->hists);
+
+	perf_hpp__for_each_format(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
+		cols += fmt->width(fmt, NULL, evsel) + 2;
+		if (cols > browser->skip_cols) {
+			browser->skip_cols = cols;
+			break;
+		}
+	}
+}
+
 static int hist_browser__run(struct hist_browser *browser, const char *help)
 {
 	int key;
@@ -494,17 +533,25 @@ static int hist_browser__run(struct hist_browser *browser, const char *help)
 			browser->show_headers = !browser->show_headers;
 			hist_browser__update_rows(browser);
 			break;
-		case '>':
+		case '.':
 			browser->skip_cols += SKIP_COLS_STEP;
 			browser->b.navkeypressed = true;
 			break;
-		case '<':
+		case ',':
 			if (browser->skip_cols > SKIP_COLS_STEP)
 				browser->skip_cols -= SKIP_COLS_STEP;
 			else
 				browser->skip_cols = 0;
 			browser->b.navkeypressed = true;
 			break;
+		case '>':
+			hist_browser__next_skip_cols(browser);
+			browser->b.navkeypressed = true;
+			break;
+		case '<':
+			hist_browser__prev_skip_cols(browser);
+			browser->b.navkeypressed = true;
+			break;
 		case K_ENTER:
 			if (hist_browser__toggle_fold(browser))
 				break;
-- 
2.5.0









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