[vitunes] Re: [PATCH] add visual mode to vitunes

  • From: Ryan Flannery <ryan.flannery@xxxxxxxxx>
  • To: vitunes@xxxxxxxxxxxxx
  • Date: Fri, 11 Feb 2011 14:42:32 -0500

Hi list,

On Fri, Feb 11, 2011 at 12:38 PM, Daniel Walter <sahne@xxxxxxx> wrote:
> On Fri, Feb 11, 2011 at 11:33:04AM +0000, Daniel Walter wrote:
>> Hi all,
>>
>> I gave your implementation a second thought and I've come up with
>> a few issues we should discuss.
>>
>> If we use your posted approach, I think we would need to modify the
>> KeyActionHandler struct with a bool to state if this command can
>> be used while in visual mode.
>>
>> e.g.: One must not switch windows while in visual, or display the
>> file meta-data. a shell or a command may make sense if it supports
>> piping the selected files, but for now it would not make much sense.
>>
>> If we use my approach, we would have to duplicate too much code for
>> key handling, so I'd say this is a no-go.
>>
>> Since I'm currently not that familiar with the implications of adding a
>> bool to the KeyActionHandler struct, I'd like to know what you think about
>> it. As far as I can see this would be the easiest way to check if a
>> given action can handle visual mode (eg. yank, cut, scrolling, jump, etc)
>> or not (switching windows, media_*, shell, command, seek_*, toggle,etc.)
>>
>> I'll post a patch with a modified KeyActionHandler and the needed changes
>> to keybinding.c later today for review.
>>
>> comments, suggestions, critics are welcome.
>>
>> daniel
>> <comments cut>
>
> Hi,
>
> as I promised, here is the patch for the enhanced visual mode for vitunes 
> based on
> ryans suggestions. I've added an additional boolean to the KeyActionHandler 
> struct
> to identify if it is usable within visual mode.
>
> Currently all scroll commands, search command, yank and cut are enabled in 
> visual mode.
> I've also added a special handling of the ESC key while in visual (it simply 
> end visual
> mode).
>
> I've compile tested it on Debian, and tested it on OS X.
>
> as usual comments are welcome.

Awesome... I've been testing it on OpenBSD for a while, and just
committed and push'd it out to the repo's.

Very nice!

I only changed two things:
1. I moved the 'bool visual' to before the Args structure, to try and
line-up that table in keybindings.c.
2. I re-added the changes from Kilian in switch_windows that keeps
only the current row in the active window drawn.

I like #2 since it makes it obvious what window we're in.  I plan to
add a colorable-item for "current row in inactive window", so one
could reproduce the old behavior, if desired (or have the current row
in the inactive window a gray).

Anyway, this is awesome!  Much thanks!
-Ryan

Other related posts: