From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754009AbbHLF5j (ORCPT ); Wed, 12 Aug 2015 01:57:39 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:36280 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753664AbbHLF5h (ORCPT ); Wed, 12 Aug 2015 01:57:37 -0400 Date: Wed, 12 Aug 2015 14:41:22 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Jiri Olsa , Ingo Molnar , Peter Zijlstra , LKML , David Ahern , Andi Kleen Subject: Re: [PATCH v2 1/2] perf hists browser: Support horizontal scrolling with '<' and '>' key Message-ID: <20150812054120.GA25642@sejong> References: <20150809093024.GA17069@krava.brq.redhat.com> <20150809103542.GA3415@danjae.kornet> <20150809112139.GD17069@krava.brq.redhat.com> <20150810155004.GB2521@kernel.org> <20150810224603.GC27431@krava.redhat.com> <20150810225822.GH2521@kernel.org> <20150810230256.GA27382@krava.redhat.com> <20150810231445.GI2521@kernel.org> <20150810231559.GB27382@krava.redhat.com> <20150811205928.GA31059@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150811205928.GA31059@kernel.org> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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