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

[Namhyung Kim <namhyung@kernel.org>]

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@kernel.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@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.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