public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	LKML <linux-kernel@vger.kernel.org>,
	David Ahern <dsahern@gmail.com>, Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCH v2 1/2] perf hists browser: Support horizontal scrolling with '<' and '>' key
Date: Tue, 11 Aug 2015 17:59:28 -0300	[thread overview]
Message-ID: <20150811205928.GA31059@kernel.org> (raw)
In-Reply-To: <20150810231559.GB27382@krava.redhat.com>

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

  reply	other threads:[~2015-08-11 20:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-09  8:21 [PATCH v2 1/2] perf hists browser: Support horizontal scrolling with '<' and '>' key Namhyung Kim
2015-08-09  8:21 ` [PATCH v2 2/2] perf hists browser: Support horizontal scrolling for callchains Namhyung Kim
2015-08-09  9:30 ` [PATCH v2 1/2] perf hists browser: Support horizontal scrolling with '<' and '>' key Jiri Olsa
2015-08-09 10:35   ` Namhyung Kim
2015-08-09 11:21     ` Jiri Olsa
2015-08-10 15:50       ` Arnaldo Carvalho de Melo
2015-08-10 22:46         ` Jiri Olsa
2015-08-10 22:58           ` Arnaldo Carvalho de Melo
2015-08-10 23:02             ` Jiri Olsa
2015-08-10 23:14               ` Arnaldo Carvalho de Melo
2015-08-10 23:15                 ` Jiri Olsa
2015-08-11 20:59                   ` Arnaldo Carvalho de Melo [this message]
2015-08-12  5:41                     ` Namhyung Kim
2015-08-12  9:15                       ` Jiri Olsa
2015-08-12 13:17                       ` Arnaldo Carvalho de Melo
2015-08-11  2:05                 ` Namhyung Kim
2015-08-11  2:00             ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150811205928.GA31059@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=andi@firstfloor.org \
    --cc=dsahern@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox