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

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

[Namhyung Kim]

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.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 66 +++++++++++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 13 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fa67613976a8..9f6e960be3a6 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -20,6 +20,8 @@
 #include "map.h"
 #include "annotate.h"
 
+#define SKIP_COLS_STEP  10
+
 struct hist_browser {
 	struct ui_browser   b;
 	struct hists	    *hists;
@@ -29,6 +31,7 @@ struct hist_browser {
 	struct pstack	    *pstack;
 	struct perf_session_env *env;
 	int		     print_seq;
+	int		     skip_cols;
 	bool		     show_dso;
 	bool		     show_headers;
 	float		     min_pcnt;
@@ -491,6 +494,17 @@ 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 '>':
+			browser->skip_cols += SKIP_COLS_STEP;
+			browser->b.navkeypressed = true;
+			break;
+		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 K_ENTER:
 			if (hist_browser__toggle_fold(browser))
 				break;
@@ -663,8 +677,27 @@ struct hpp_arg {
 	struct ui_browser *b;
 	char folded_sign;
 	bool current_entry;
+	int skip_cols;
 };
 
+/* returns actual printed length (not skipped) */
+static int hpp_print_skip(struct perf_hpp *hpp, char *buf, size_t size)
+{
+	struct hpp_arg *arg = hpp->ptr;
+	int ret = size;
+
+	if (ret > arg->skip_cols) {
+		slsmg_printf("%s", buf + arg->skip_cols);
+		ret -= arg->skip_cols;
+		arg->skip_cols = 0;
+	} else {
+		arg->skip_cols -= ret;
+		ret = 0;
+	}
+
+	return ret;
+}
+
 static int __hpp__slsmg_color_printf(struct perf_hpp *hpp, const char *fmt, ...)
 {
 	struct hpp_arg *arg = hpp->ptr;
@@ -680,7 +713,7 @@ static int __hpp__slsmg_color_printf(struct perf_hpp *hpp, const char *fmt, ...)
 	ui_browser__set_percent_color(arg->b, percent, arg->current_entry);
 
 	ret = scnprintf(hpp->buf, hpp->size, fmt, len, percent);
-	slsmg_printf("%s", hpp->buf);
+	ret = hpp_print_skip(hpp, hpp->buf, ret);
 
 	advance_hpp(hpp, ret);
 	return ret;
@@ -716,9 +749,8 @@ hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt,		\
 		int len = fmt->user_len ?: fmt->len;			\
 		int ret = scnprintf(hpp->buf, hpp->size,		\
 				    "%*s", len, "N/A");			\
-		slsmg_printf("%s", hpp->buf);				\
 									\
-		return ret;						\
+		return hpp_print_skip(hpp, hpp->buf, ret);		\
 	}								\
 	return hpp__fmt(fmt, hpp, he, __hpp_get_acc_##_field,		\
 			" %*.2f%%", __hpp__slsmg_color_printf, true);	\
@@ -778,6 +810,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 			.b		= &browser->b,
 			.folded_sign	= folded_sign,
 			.current_entry	= current_entry,
+			.skip_cols	= browser->skip_cols,
 		};
 		struct perf_hpp hpp = {
 			.buf		= s,
@@ -801,20 +834,20 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 
 			if (first) {
 				if (symbol_conf.use_callchain) {
-					slsmg_printf("%c ", folded_sign);
-					width -= 2;
+					snprintf(s, sizeof(s), "%c ", folded_sign);
+					width -= hpp_print_skip(&hpp, s, 2);
 				}
 				first = false;
 			} else {
-				slsmg_printf("  ");
-				width -= 2;
+				strcpy(s, "  ");
+				width -= hpp_print_skip(&hpp, s, 2);
 			}
 
 			if (fmt->color) {
 				width -= fmt->color(fmt, &hpp, entry);
 			} else {
-				width -= fmt->entry(fmt, &hpp, entry);
-				slsmg_printf("%s", s);
+				int ret = fmt->entry(fmt, &hpp, entry);
+				width -= hpp_print_skip(&hpp, s, ret);
 			}
 		}
 
@@ -873,7 +906,7 @@ static int hists__scnprintf_headers(char *buf, size_t size, struct hists *hists)
 	if (symbol_conf.use_callchain) {
 		ret = scnprintf(buf, size, "  ");
 		if (advance_hpp_check(&dummy_hpp, ret))
-			return ret;
+			goto out;
 	}
 
 	perf_hpp__for_each_format(fmt) {
@@ -889,17 +922,23 @@ static int hists__scnprintf_headers(char *buf, size_t size, struct hists *hists)
 			break;
 	}
 
-	return ret;
+out:
+	return dummy_hpp.buf - buf;
 }
 
 static void hist_browser__show_headers(struct hist_browser *browser)
 {
 	char headers[1024];
+	int width = browser->b.width + 1;
+	int ret;
 
-	hists__scnprintf_headers(headers, sizeof(headers), browser->hists);
+	ret = hists__scnprintf_headers(headers, sizeof(headers), browser->hists);
 	ui_browser__gotorc(&browser->b, 0, 0);
 	ui_browser__set_color(&browser->b, HE_COLORSET_ROOT);
-	slsmg_write_nstring(headers, browser->b.width + 1);
+	if (browser->skip_cols < ret)
+		slsmg_write_nstring(headers + browser->skip_cols, width);
+	else
+		slsmg_write_nstring("", width);
 }
 
 static void ui_browser__hists_init_top(struct ui_browser *browser)
@@ -1707,6 +1746,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 #define HIST_BROWSER_HELP_COMMON					\
 	"h/?/F1        Show this window\n"				\
 	"UP/DOWN/PGUP\n"						\
+	"</> (angle brackets)\n"					\
 	"PGDN/SPACE    Navigate\n"					\
 	"q/ESC/CTRL+C  Exit browser\n\n"				\
 	"For multiple event sessions:\n\n"				\
-- 
2.5.0

[PATCH v2 2/2] perf hists browser: Support horizontal scrolling for callchains

[Namhyung Kim]

In the previous patch, horizontal scrolling was added to TUI browser.
However it lacks support for callchains.  This patch adds it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 46 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 9f6e960be3a6..10af42f5283e 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -526,6 +526,7 @@ struct callchain_print_arg {
 	/* for file dump */
 	FILE	*fp;
 	int	printed;
+	int	skip_cols;
 };
 
 typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
@@ -534,6 +535,30 @@ typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
 					 unsigned short row,
 					 struct callchain_print_arg *arg);
 
+static int callchain_print_skip(struct callchain_print_arg *arg,
+				const char *str, int width)
+{
+	if (arg->skip_cols) {
+		int len = strlen(str);
+
+		if (width <= arg->skip_cols) {
+			arg->skip_cols -= width;
+			return 0;
+		}
+
+		if (arg->skip_cols < len)
+			str += arg->skip_cols;
+		else
+			str = " ";
+
+		width -= arg->skip_cols;
+		arg->skip_cols = 0;
+	}
+
+	slsmg_write_nstring(str, width);
+	return width;
+}
+
 static void hist_browser__show_callchain_entry(struct hist_browser *browser,
 					       struct callchain_list *chain,
 					       const char *str, int offset,
@@ -543,9 +568,10 @@ static void hist_browser__show_callchain_entry(struct hist_browser *browser,
 	int color, width;
 	char folded_sign = callchain_list__folded(chain);
 	bool show_annotated = browser->show_dso && chain->ms.sym && symbol__annotation(chain->ms.sym)->src;
+	char buf[2] = { folded_sign, 0 };
 
 	color = HE_COLORSET_NORMAL;
-	width = browser->b.width - (offset + 2);
+	width = browser->b.width;
 	if (ui_browser__is_current_entry(&browser->b, row)) {
 		browser->selection = &chain->ms;
 		color = HE_COLORSET_SELECTED;
@@ -554,10 +580,17 @@ static void hist_browser__show_callchain_entry(struct hist_browser *browser,
 
 	ui_browser__set_color(&browser->b, color);
 	hist_browser__gotorc(browser, row, 0);
-	slsmg_write_nstring(" ", offset);
-	slsmg_printf("%c", folded_sign);
-	ui_browser__write_graph(&browser->b, show_annotated ? SLSMG_RARROW_CHAR : ' ');
-	slsmg_write_nstring(str, width);
+	width -= callchain_print_skip(arg, " ", offset);
+	width -= callchain_print_skip(arg, buf, 1);
+	if (arg->skip_cols > 0)
+		arg->skip_cols--;
+	else {
+		ui_browser__write_graph(&browser->b,
+					show_annotated ? SLSMG_RARROW_CHAR : ' ');
+		width--;
+	}
+	width -= callchain_print_skip(arg, str, width);
+	slsmg_write_nstring("", width);
 }
 
 static void hist_browser__fprintf_callchain_entry(struct hist_browser *b __maybe_unused,
@@ -642,6 +675,7 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 					str = alloc_str;
 			}
 
+			arg->skip_cols = browser->skip_cols;
 			print(browser, chain, str, offset + extra_offset, row, arg);
 
 			free(alloc_str);
@@ -661,6 +695,7 @@ do_next:
 			else
 				new_total = total;
 
+			arg->skip_cols = browser->skip_cols;
 			row += hist_browser__show_callchain(browser, &child->rb_root,
 							    new_level, row, new_total,
 							    print, arg, is_output_full);
@@ -867,6 +902,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		struct callchain_print_arg arg = {
 			.row_offset = row_offset,
 			.is_current_entry = current_entry,
+			.skip_cols = browser->skip_cols,
 		};
 
 		if (callchain_param.mode == CHAIN_GRAPH_REL) {
-- 
2.5.0

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

[Jiri Olsa]

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 ;-)

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

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

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

jirka

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

[Namhyung Kim]

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

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

[Jiri Olsa]

On Sun, Aug 09, 2015 at 07:35:42PM +0900, Namhyung Kim wrote:
> 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.

I like it ;-)

Tested-by: Jiri Olsa <jolsa@redhat.com>

anyway let's hear some other opinions

thanks,
jirka

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

[Arnaldo Carvalho de Melo]

Em Sun, Aug 09, 2015 at 01:21:39PM +0200, Jiri Olsa escreveu:
> On Sun, Aug 09, 2015 at 07:35:42PM +0900, Namhyung Kim wrote:
> > 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.

> > 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.
> 
> I like it ;-)

Humm, I don't, its not natural, I doubt anyone will think about using ,
and . to move by one character right/left, ditto for <>, I think we
should just recover <- and -> (left and right arrows) for navigation,
leave ENTER to be what -> was doing and esc for what <- does, in fact it
is already like that.

Probably using shift or control or alt + <- -> for moving one column at
at time is enough, but humm, is there value in that?

I'll try the patch and will try to do the changes as I described, will
post here.
 
> Tested-by: Jiri Olsa <jolsa@redhat.com>
> 
> anyway let's hear some other opinions
> 
> thanks,
> jirka

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

[Jiri Olsa]

On Mon, Aug 10, 2015 at 12:50:04PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Aug 09, 2015 at 01:21:39PM +0200, Jiri Olsa escreveu:
> > On Sun, Aug 09, 2015 at 07:35:42PM +0900, Namhyung Kim wrote:
> > > 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.
> 
> > > 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.
> > 
> > I like it ;-)
> 
> Humm, I don't, its not natural, I doubt anyone will think about using ,
> and . to move by one character right/left, ditto for <>, I think we
> should just recover <- and -> (left and right arrows) for navigation,
> leave ENTER to be what -> was doing and esc for what <- does, in fact it
> is already like that.

I would ;-) anyway I also think arrows are the best, but I thought we
dont want to break existing interface much

> 
> Probably using shift or control or alt + <- -> for moving one column at
> at time is enough, but humm, is there value in that?

it'd really help for perf mem output, which is quite wide.. and we probably
have more wide outputs, or users with narrow terminals ;-)

jirka

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

[Arnaldo Carvalho de Melo]

Em Tue, Aug 11, 2015 at 12:46:03AM +0200, Jiri Olsa escreveu:
> On Mon, Aug 10, 2015 at 12:50:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sun, Aug 09, 2015 at 01:21:39PM +0200, Jiri Olsa escreveu:
> > > On Sun, Aug 09, 2015 at 07:35:42PM +0900, Namhyung Kim wrote:
> > > > 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.
> > 
> > > > 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.
> > > 
> > > I like it ;-)
> > 
> > Humm, I don't, its not natural, I doubt anyone will think about using ,
> > and . to move by one character right/left, ditto for <>, I think we
> > should just recover <- and -> (left and right arrows) for navigation,
> > leave ENTER to be what -> was doing and esc for what <- does, in fact it
> > is already like that.
> 
> I would ;-) anyway I also think arrows are the best, but I thought we
> dont want to break existing interface much

In this case I think we can, enter and esc are really well know keys for
entering and escaping from a place, and we have them working forever
doing that, its just that at some time I used <- as an alias for ESC and
-> as alias for ENTER.

I.e. for people using ESC and ENTER, this will not be noticed :-)
 
> > Probably using shift or control or alt + <- -> for moving one column at
> > at time is enough, but humm, is there value in that?
> 
> it'd really help for perf mem output, which is quite wide.. and we probably
> have more wide outputs, or users with narrow terminals ;-)

I'm not going against horizontal scrolling, it is needed, sure thing,
its surprising we are doing this only now. What I am asking is this fine
scrolling of one column per <- or -> keypress, but I really need to try
it with things like 'perf mem', I thought that when you pressed '>', in
this patch, it would move entire sort key columns, not just one vertical
column one character wide, right?

- Arnaldo

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

[Jiri Olsa]

On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > it'd really help for perf mem output, which is quite wide.. and we probably
> > have more wide outputs, or users with narrow terminals ;-)
> 
> I'm not going against horizontal scrolling, it is needed, sure thing,
> its surprising we are doing this only now. What I am asking is this fine
> scrolling of one column per <- or -> keypress, but I really need to try
> it with things like 'perf mem', I thought that when you pressed '>', in
> this patch, it would move entire sort key columns, not just one vertical
> column one character wide, right?

yep, the whole sort column seemed more usefull,
though 1 vertical column is nice also

jirka

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

[Arnaldo Carvalho de Melo]

Em Tue, Aug 11, 2015 at 01:02:56AM +0200, Jiri Olsa escreveu:
> On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:
 
> SNIP
 
> > > it'd really help for perf mem output, which is quite wide.. and we probably
> > > have more wide outputs, or users with narrow terminals ;-)

> > I'm not going against horizontal scrolling, it is needed, sure thing,
> > its surprising we are doing this only now. What I am asking is this fine
> > scrolling of one column per <- or -> keypress, but I really need to try
> > it with things like 'perf mem', I thought that when you pressed '>', in
> > this patch, it would move entire sort key columns, not just one vertical
> > column one character wide, right?
 
> yep, the whole sort column seemed more usefull,
> though 1 vertical column is nice also

We can get that by using the right and left keys while pressing shift or
alt, I think.

- Arnaldo

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

[Jiri Olsa]

On Mon, Aug 10, 2015 at 08:14:45PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 11, 2015 at 01:02:56AM +0200, Jiri Olsa escreveu:
> > On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:
>  
> > SNIP
>  
> > > > it'd really help for perf mem output, which is quite wide.. and we probably
> > > > have more wide outputs, or users with narrow terminals ;-)
> 
> > > I'm not going against horizontal scrolling, it is needed, sure thing,
> > > its surprising we are doing this only now. What I am asking is this fine
> > > scrolling of one column per <- or -> keypress, but I really need to try
> > > it with things like 'perf mem', I thought that when you pressed '>', in
> > > this patch, it would move entire sort key columns, not just one vertical
> > > column one character wide, right?
>  
> > yep, the whole sort column seemed more usefull,
> > though 1 vertical column is nice also
> 
> We can get that by using the right and left keys while pressing shift or
> alt, I think.

works for me

jirka

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

[Arnaldo Carvalho de Melo]

Em Tue, Aug 11, 2015 at 01:15:59AM +0200, Jiri Olsa escreveu:
> On Mon, Aug 10, 2015 at 08:14:45PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Aug 11, 2015 at 01:02:56AM +0200, Jiri Olsa escreveu:
> > > On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > it'd really help for perf mem output, which is quite wide.. and we probably
> > > > > have more wide outputs, or users with narrow terminals ;-)

> > > > I'm not going against horizontal scrolling, it is needed, sure thing,
> > > > its surprising we are doing this only now. What I am asking is this fine
> > > > scrolling of one column per <- or -> keypress, but I really need to try
> > > > it with things like 'perf mem', I thought that when you pressed '>', in
> > > > this patch, it would move entire sort key columns, not just one vertical
> > > > column one character wide, right?

> > > yep, the whole sort column seemed more usefull,

So, that wasn't what was implemented in Namhyung's patchkit, right? I.e.
it scrolls characters, not columns.

Can you take a look at the patchkit I put together at my tree, branch:

	tmp.perf/ui_browser.horiz_scroll

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=tmp.perf/ui_browser.horiz_scroll

The last two patches do it:

  $ git log --oneline | head -2
  8a7684de198b perf hists browser: Implement horizontal scrolling
  a00e506da09e perf ui browser: Optional horizontal scrolling key binding
  $

  $ git diff HEAD^^ --stat
   tools/perf/ui/browser.c        | 14 ++++++++++++++
   tools/perf/ui/browser.h        |  2 +-
   tools/perf/ui/browsers/hists.c | 22 +++++++++++++++++-----
   3 files changed, 32 insertions(+), 6 deletions(-)
  $

Several lines are just comments explaining some tricks due to me not
having found a counter for the number of colums somewhere and reusing
the first loop that traverses them all to do the counting.

There are two before those that at first I thought was needed, but ended
up not using (would have to render the whole line in ui_browser to do
the scrolling at line printing time, works only for browsers where just
one call to ui_browser__printf or ui_browser__write_nstring is done,
but I ended up leaving it there anyway, to try to make the
hist_browser.c and other ui_browser implementations (annotate, etc)
independent of libslang:

  $ git log --oneline | head -4 | tail -2
  e7534e88dfa3 perf ui browser: Introduce ui_browser__printf()
  2fe0f7e4b73e perf ui browser: Introduce ui_browser__write_nstring()
  $

Tested it with 'perf mem record -g -a' + 'perf mem report', and I liked
how it works, please check if you like it too :-)

The <- and -> keys are reused just when the horizontal scrolling mode is
activate by setting ui_browser->columns, the hists_browser (perf report,
perf top) will continue having ENTER and ESC, as always, to
select/deselect things.

If we insist we need character by character scrolling, or if we need
that move in other browser (annotate, for instance) its just a matter of
using ui_browser->horiz_scroll as a char counter and use it in the
ui_browser->refresh() calls when rendering each line.

- Arnaldo

> > > though 1 vertical column is nice also
> > 
> > We can get that by using the right and left keys while pressing shift or
> > alt, I think.
> 
> works for me
> 
> jirka

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

[Namhyung Kim]

Hi Arnaldo,

On Tue, Aug 11, 2015 at 05:59:28PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 11, 2015 at 01:15:59AM +0200, Jiri Olsa escreveu:
> > On Mon, Aug 10, 2015 at 08:14:45PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Aug 11, 2015 at 01:02:56AM +0200, Jiri Olsa escreveu:
> > > > On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > > it'd really help for perf mem output, which is quite wide.. and we probably
> > > > > > have more wide outputs, or users with narrow terminals ;-)
> 
> > > > > I'm not going against horizontal scrolling, it is needed, sure thing,
> > > > > its surprising we are doing this only now. What I am asking is this fine
> > > > > scrolling of one column per <- or -> keypress, but I really need to try
> > > > > it with things like 'perf mem', I thought that when you pressed '>', in
> > > > > this patch, it would move entire sort key columns, not just one vertical
> > > > > column one character wide, right?
> 
> > > > yep, the whole sort column seemed more usefull,
> 
> So, that wasn't what was implemented in Namhyung's patchkit, right? I.e.
> it scrolls characters, not columns.

My last patch already implemented the scrolling by columns as well as
characters.


> 
> Can you take a look at the patchkit I put together at my tree, branch:
> 
> 	tmp.perf/ui_browser.horiz_scroll
> 
> https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=tmp.perf/ui_browser.horiz_scroll
> 
> The last two patches do it:
> 
>   $ git log --oneline | head -2

You can simply use 'git log --oneline -2'. :)


>   8a7684de198b perf hists browser: Implement horizontal scrolling
>   a00e506da09e perf ui browser: Optional horizontal scrolling key binding
>   $
> 
>   $ git diff HEAD^^ --stat
>    tools/perf/ui/browser.c        | 14 ++++++++++++++
>    tools/perf/ui/browser.h        |  2 +-
>    tools/perf/ui/browsers/hists.c | 22 +++++++++++++++++-----
>    3 files changed, 32 insertions(+), 6 deletions(-)
>   $

Looks good.  It's much simpler than mine and I think it's good if we
decided to support only column scrolling.


> 
> Several lines are just comments explaining some tricks due to me not
> having found a counter for the number of colums somewhere and reusing
> the first loop that traverses them all to do the counting.
> 
> There are two before those that at first I thought was needed, but ended
> up not using (would have to render the whole line in ui_browser to do
> the scrolling at line printing time, works only for browsers where just
> one call to ui_browser__printf or ui_browser__write_nstring is done,
> but I ended up leaving it there anyway, to try to make the
> hist_browser.c and other ui_browser implementations (annotate, etc)
> independent of libslang:
> 
>   $ git log --oneline | head -4 | tail -2
>   e7534e88dfa3 perf ui browser: Introduce ui_browser__printf()
>   2fe0f7e4b73e perf ui browser: Introduce ui_browser__write_nstring()
>   $
> 
> Tested it with 'perf mem record -g -a' + 'perf mem report', and I liked
> how it works, please check if you like it too :-)
> 
> The <- and -> keys are reused just when the horizontal scrolling mode is
> activate by setting ui_browser->columns, the hists_browser (perf report,
> perf top) will continue having ENTER and ESC, as always, to
> select/deselect things.

Currently the help message in the hist browser says the arrows keys
are used to zoom in & out and ESC is for 'exit browser'.  Do you think
it's ok to change the current behavior?


> 
> If we insist we need character by character scrolling, or if we need
> that move in other browser (annotate, for instance) its just a matter of
> using ui_browser->horiz_scroll as a char counter and use it in the
> ui_browser->refresh() calls when rendering each line.

Yes, I think it's needed and it'd be great if other browsers support it.

Thanks,
Namhyung

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

[Jiri Olsa]

On Wed, Aug 12, 2015 at 02:41:22PM +0900, Namhyung Kim wrote:

SNIP

> > 
> > Several lines are just comments explaining some tricks due to me not
> > having found a counter for the number of colums somewhere and reusing
> > the first loop that traverses them all to do the counting.
> > 
> > There are two before those that at first I thought was needed, but ended
> > up not using (would have to render the whole line in ui_browser to do
> > the scrolling at line printing time, works only for browsers where just
> > one call to ui_browser__printf or ui_browser__write_nstring is done,
> > but I ended up leaving it there anyway, to try to make the
> > hist_browser.c and other ui_browser implementations (annotate, etc)
> > independent of libslang:
> > 
> >   $ git log --oneline | head -4 | tail -2
> >   e7534e88dfa3 perf ui browser: Introduce ui_browser__printf()
> >   2fe0f7e4b73e perf ui browser: Introduce ui_browser__write_nstring()
> >   $
> > 
> > Tested it with 'perf mem record -g -a' + 'perf mem report', and I liked
> > how it works, please check if you like it too :-)
> > 
> > The <- and -> keys are reused just when the horizontal scrolling mode is
> > activate by setting ui_browser->columns, the hists_browser (perf report,
> > perf top) will continue having ENTER and ESC, as always, to
> > select/deselect things.
> 
> Currently the help message in the hist browser says the arrows keys
> are used to zoom in & out and ESC is for 'exit browser'.  Do you think
> it's ok to change the current behavior?

I have some concern here as well.. but let's try and see ;-)

but if we go this way I think I'd slighly prefer following behaviour:
       arrows : scroll by 1 column
 SHIFT-arrows : scroll by sort column

but it's just a suggestion, I like your change as it is now as well

thanks,
jirka

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

[Arnaldo Carvalho de Melo]

Em Wed, Aug 12, 2015 at 02:41:22PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> On Tue, Aug 11, 2015 at 05:59:28PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Aug 11, 2015 at 01:15:59AM +0200, Jiri Olsa escreveu:
> > > On Mon, Aug 10, 2015 at 08:14:45PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Tue, Aug 11, 2015 at 01:02:56AM +0200, Jiri Olsa escreveu:
> > > > > On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > > > it'd really help for perf mem output, which is quite wide.. and we probably
> > > > > > > have more wide outputs, or users with narrow terminals ;-)

> > > > > > I'm not going against horizontal scrolling, it is needed, sure thing,
> > > > > > its surprising we are doing this only now. What I am asking is this fine
> > > > > > scrolling of one column per <- or -> keypress, but I really need to try
> > > > > > it with things like 'perf mem', I thought that when you pressed '>', in
> > > > > > this patch, it would move entire sort key columns, not just one vertical
> > > > > > column one character wide, right?

> > > > > yep, the whole sort column seemed more usefull,

> > So, that wasn't what was implemented in Namhyung's patchkit, right? I.e.
> > it scrolls characters, not columns.
 
> My last patch already implemented the scrolling by columns as well as
> characters.

Oh, didn't notice, should have read it more carefully :-\
 
> > Can you take a look at the patchkit I put together at my tree, branch:
> > 
> > 	tmp.perf/ui_browser.horiz_scroll
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/log/?h=tmp.perf/ui_browser.horiz_scroll
> > 
> > The last two patches do it:
> > 
> >   $ git log --oneline | head -2
> 
> You can simply use 'git log --oneline -2'. :)

thanks :)
 
> >   8a7684de198b perf hists browser: Implement horizontal scrolling
> >   a00e506da09e perf ui browser: Optional horizontal scrolling key binding
> >   $
> > 
> >   $ git diff HEAD^^ --stat
> >    tools/perf/ui/browser.c        | 14 ++++++++++++++
> >    tools/perf/ui/browser.h        |  2 +-
> >    tools/perf/ui/browsers/hists.c | 22 +++++++++++++++++-----
> >    3 files changed, 32 insertions(+), 6 deletions(-)
> >   $
> 
> Looks good.  It's much simpler than mine and I think it's good if we
> decided to support only column scrolling.

Even if we decide to support both, which I agree, this way of
implementing horizontal scrolling, as you said, is really simple, and
all browsers wanting horizontal scrolling will want these basic blocks,
see below for char by char horizontal scrolling.
 
> > Several lines are just comments explaining some tricks due to me not
> > having found a counter for the number of colums somewhere and reusing
> > the first loop that traverses them all to do the counting.

> > There are two before those that at first I thought was needed, but ended
> > up not using (would have to render the whole line in ui_browser to do
> > the scrolling at line printing time, works only for browsers where just
> > one call to ui_browser__printf or ui_browser__write_nstring is done,
> > but I ended up leaving it there anyway, to try to make the
> > hist_browser.c and other ui_browser implementations (annotate, etc)
> > independent of libslang:

> >   $ git log --oneline | head -4 | tail -2
> >   e7534e88dfa3 perf ui browser: Introduce ui_browser__printf()
> >   2fe0f7e4b73e perf ui browser: Introduce ui_browser__write_nstring()

> > Tested it with 'perf mem record -g -a' + 'perf mem report', and I liked
> > how it works, please check if you like it too :-)
> > 
> > The <- and -> keys are reused just when the horizontal scrolling mode is
> > activate by setting ui_browser->columns, the hists_browser (perf report,
> > perf top) will continue having ENTER and ESC, as always, to
> > select/deselect things.
 
> Currently the help message in the hist browser says the arrows keys
> are used to zoom in & out and ESC is for 'exit browser'.  Do you think
> it's ok to change the current behavior?

I think it is.
 
> > If we insist we need character by character scrolling, or if we need
> > that move in other browser (annotate, for instance) its just a matter of
> > using ui_browser->horiz_scroll as a char counter and use it in the
> > ui_browser->refresh() calls when rendering each line.
> 
> Yes, I think it's needed and it'd be great if other browsers support it.

So, with ui_browser__write_nstring(), ui_browser__printf() wrapped,
we're left with the other methods dealing with graph characters to
support char-by-char working for horizontal scrolling.

What I think we should do is to buffer all the updates to the screen,
then, right after returning from ui_browser->refresh(), traverse the
list of formatted lines and chop off ui_browser->horiz_scroll from the
start of each line.

But this requires a bit more work, to think about gotorc, etc, which I
think can be left for later, as column scrolling in the simple way done
now seems to serve us well for things like 'perf mem report', which I
think is _the_ use case now, right?

What I don't want to see is us overcomplicating browser by browser to do
things that all of them need to do :-\

With regard to using Alt+-> for column by column and -> for
char by char horizontal scrolling, I think we can have instead a hotkey
we press to toggle the scrolling mode, i.e. we start with either way,
the one considered most useful (column by column IMNSHO) and if we
press, say, '.', the we switch to using the other one (char by char),
the behaviour could be selected by ~/.perfconfig setting too, of course.

- Arnald

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

[Namhyung Kim]

On Mon, Aug 10, 2015 at 08:14:45PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 11, 2015 at 01:02:56AM +0200, Jiri Olsa escreveu:
> > On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:
>  
> > SNIP
>  
> > > > it'd really help for perf mem output, which is quite wide.. and we probably
> > > > have more wide outputs, or users with narrow terminals ;-)
> 
> > > I'm not going against horizontal scrolling, it is needed, sure thing,
> > > its surprising we are doing this only now. What I am asking is this fine
> > > scrolling of one column per <- or -> keypress, but I really need to try
> > > it with things like 'perf mem', I thought that when you pressed '>', in
> > > this patch, it would move entire sort key columns, not just one vertical
> > > column one character wide, right?
>  
> > yep, the whole sort column seemed more usefull,
> > though 1 vertical column is nice also
> 
> We can get that by using the right and left keys while pressing shift or
> alt, I think.

I'd also like to do it.  But it seems SLang_getkey() doesn't return
the shift, alt or control key status for arrow keys.  CTRL(K_LEFT) or
META(K_RIGHT) didn't work for me.  Anyone has an idea?

Thanks,
Namhyung

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

[Namhyung Kim]

Hi Arnaldo,

On Mon, Aug 10, 2015 at 07:58:22PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 11, 2015 at 12:46:03AM +0200, Jiri Olsa escreveu:
> > On Mon, Aug 10, 2015 at 12:50:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Sun, Aug 09, 2015 at 01:21:39PM +0200, Jiri Olsa escreveu:
> > > > On Sun, Aug 09, 2015 at 07:35:42PM +0900, Namhyung Kim wrote:
> > > > > 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.
> > > > 
> > > > I like it ;-)
> > > 
> > > Humm, I don't, its not natural, I doubt anyone will think about using ,
> > > and . to move by one character right/left, ditto for <>, I think we
> > > should just recover <- and -> (left and right arrows) for navigation,
> > > leave ENTER to be what -> was doing and esc for what <- does, in fact it
> > > is already like that.
> > 
> > I would ;-) anyway I also think arrows are the best, but I thought we
> > dont want to break existing interface much
> 
> In this case I think we can, enter and esc are really well know keys for
> entering and escaping from a place, and we have them working forever
> doing that, its just that at some time I used <- as an alias for ESC and
> -> as alias for ENTER.
> 
> I.e. for people using ESC and ENTER, this will not be noticed :-)

Yes, but I guess many people use arrow keys more than ESC and ENTER
since we use up/down arrow keys anyway.  I do want to use left/right
arrow for scrolling, but I'm afraid that it'll bother existing users..

Thanks,
Namhyung


>  
> > > Probably using shift or control or alt + <- -> for moving one column at
> > > at time is enough, but humm, is there value in that?
> > 
> > it'd really help for perf mem output, which is quite wide.. and we probably
> > have more wide outputs, or users with narrow terminals ;-)
> 
> I'm not going against horizontal scrolling, it is needed, sure thing,
> its surprising we are doing this only now. What I am asking is this fine
> scrolling of one column per <- or -> keypress, but I really need to try
> it with things like 'perf mem', I thought that when you pressed '>', in
> this patch, it would move entire sort key columns, not just one vertical
> column one character wide, right?
> 
> - Arnaldo