[vitunes] Re: [PATCH] add visual mode to vitunes
- From: Daniel Walter <sahne@xxxxxxx>
- To: vitunes@xxxxxxxxxxxxx
- Date: Fri, 11 Feb 2011 17:38:27 +0000
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.
daniel
diff -r 3418951e8398 keybindings.c
--- a/keybindings.c Thu Feb 10 16:46:22 2011 -0500
+++ b/keybindings.c Fri Feb 11 18:26:29 2011 +0100
@@ -45,6 +45,7 @@
{ search_backward, "search_backward" },
{ find_next_forward, "find_next_forward" },
{ find_next_backward, "find_next_backward" },
+ { visual, "visual" },
{ cut, "cut" },
{ yank, "yank" },
{ paste_after, "paste_after" },
@@ -78,55 +79,57 @@
KeyAction action;
ActionHandler handler;
KbaArgs args;
+ bool visual;
} KeyActionHandler;
#define ARG_NOT_USED { .num = 0 }
const KeyActionHandler KeyActionHandlers[] = {
- { scroll_up, kba_scroll_row, { .direction = UP }},
- { scroll_down, kba_scroll_row, { .direction = DOWN }},
- { scroll_up_page, kba_scroll_page, { .direction = UP, .amount =
SINGLE }},
- { scroll_down_page, kba_scroll_page, { .direction = DOWN, .amount =
SINGLE }},
- { scroll_up_halfpage, kba_scroll_page, { .direction = UP, .amount =
HALF }},
- { scroll_down_halfpage, kba_scroll_page, { .direction = DOWN, .amount =
HALF }},
- { scroll_up_wholepage, kba_scroll_page, { .direction = UP, .amount =
WHOLE }},
- { scroll_down_wholepage, kba_scroll_page, { .direction = DOWN, .amount =
WHOLE }},
- { scroll_left, kba_scroll_col, { .direction = LEFT, .amount
= SINGLE }},
- { scroll_right, kba_scroll_col, { .direction = RIGHT, .amount
= SINGLE }},
- { scroll_leftmost, kba_scroll_col, { .direction = LEFT, .amount
= WHOLE }},
- { scroll_rightmost, kba_scroll_col, { .direction = RIGHT, .amount
= WHOLE }},
- { jumpto_screen_top, kba_jumpto_screen,{ .placement = TOP }},
- { jumpto_screen_middle, kba_jumpto_screen,{ .placement = MIDDLE }},
- { jumpto_screen_bottom, kba_jumpto_screen,{ .placement = BOTTOM }},
- { jumpto_line, kba_jumpto_file, { .scale = NUMBER, .num = 'G'
}},
- { jumpto_percent, kba_jumpto_file, { .scale = PERCENT }},
- { search_forward, kba_search, { .direction = FORWARDS }},
- { search_backward, kba_search, { .direction = BACKWARDS }},
- { find_next_forward, kba_search_find, { .direction = SAME }},
- { find_next_backward, kba_search_find, { .direction = OPPOSITE }},
- { cut, kba_cut, ARG_NOT_USED },
- { yank, kba_yank, ARG_NOT_USED },
- { paste_after, kba_paste, { .placement = AFTER }},
- { paste_before, kba_paste, { .placement = BEFORE }},
- { undo, kba_undo, ARG_NOT_USED },
- { redo, kba_redo, ARG_NOT_USED },
- { go, kba_go, ARG_NOT_USED },
- { quit, kba_quit, ARG_NOT_USED },
- { redraw, kba_redraw, ARG_NOT_USED },
- { command_mode, kba_command_mode, ARG_NOT_USED },
- { shell, kba_shell, ARG_NOT_USED },
- { switch_windows, kba_switch_windows, ARG_NOT_USED },
- { show_file_info, kba_show_file_info, ARG_NOT_USED },
- { load_playlist, kba_load_playlist, ARG_NOT_USED },
- { media_play, kba_play, ARG_NOT_USED },
- { media_pause, kba_pause, ARG_NOT_USED },
- { media_stop, kba_stop, ARG_NOT_USED },
- { media_next, kba_play_next, ARG_NOT_USED },
- { media_prev, kba_play_prev, ARG_NOT_USED },
- { seek_forward_seconds, kba_seek, { .direction = FORWARDS, .scale =
SECONDS, .num = 10 }},
- { seek_backward_seconds, kba_seek, { .direction = BACKWARDS, .scale =
SECONDS, .num = 10 }},
- { seek_forward_minutes, kba_seek, { .direction = FORWARDS, .scale =
MINUTES, .num = 1 }},
- { seek_backward_minutes, kba_seek, { .direction = BACKWARDS, .scale =
MINUTES, .num = 1 }},
- { toggle, kba_toggle, { .scale = NUMBER, .num = 'G' }}
+ { scroll_up, kba_scroll_row, { .direction = UP }, true},
+ { scroll_down, kba_scroll_row, { .direction = DOWN }, true},
+ { scroll_up_page, kba_scroll_page, { .direction = UP, .amount =
SINGLE }, true},
+ { scroll_down_page, kba_scroll_page, { .direction = DOWN, .amount =
SINGLE }, true},
+ { scroll_up_halfpage, kba_scroll_page, { .direction = UP, .amount =
HALF }, true},
+ { scroll_down_halfpage, kba_scroll_page, { .direction = DOWN, .amount =
HALF }, true},
+ { scroll_up_wholepage, kba_scroll_page, { .direction = UP, .amount =
WHOLE }, true},
+ { scroll_down_wholepage, kba_scroll_page, { .direction = DOWN, .amount =
WHOLE }, true},
+ { scroll_left, kba_scroll_col, { .direction = LEFT, .amount
= SINGLE }, true},
+ { scroll_right, kba_scroll_col, { .direction = RIGHT, .amount
= SINGLE }, true},
+ { scroll_leftmost, kba_scroll_col, { .direction = LEFT, .amount
= WHOLE }, true},
+ { scroll_rightmost, kba_scroll_col, { .direction = RIGHT, .amount
= WHOLE }, true},
+ { jumpto_screen_top, kba_jumpto_screen,{ .placement = TOP }, true},
+ { jumpto_screen_middle, kba_jumpto_screen,{ .placement = MIDDLE }, true},
+ { jumpto_screen_bottom, kba_jumpto_screen,{ .placement = BOTTOM }, true},
+ { jumpto_line, kba_jumpto_file, { .scale = NUMBER, .num = 'G'
}, true},
+ { jumpto_percent, kba_jumpto_file, { .scale = PERCENT }, true},
+ { search_forward, kba_search, { .direction = FORWARDS },
true},
+ { search_backward, kba_search, { .direction = BACKWARDS },
true},
+ { find_next_forward, kba_search_find, { .direction = SAME }, true},
+ { find_next_backward, kba_search_find, { .direction = OPPOSITE },
true},
+ { visual, kba_visual, ARG_NOT_USED , true},
+ { cut, kba_cut, ARG_NOT_USED , true},
+ { yank, kba_yank, ARG_NOT_USED , true},
+ { paste_after, kba_paste, { .placement = AFTER }, false},
+ { paste_before, kba_paste, { .placement = BEFORE },
false},
+ { undo, kba_undo, ARG_NOT_USED , false},
+ { redo, kba_redo, ARG_NOT_USED , false},
+ { go, kba_go, ARG_NOT_USED , true},
+ { quit, kba_quit, ARG_NOT_USED , false},
+ { redraw, kba_redraw, ARG_NOT_USED , false},
+ { command_mode, kba_command_mode, ARG_NOT_USED , false},
+ { shell, kba_shell, ARG_NOT_USED , false},
+ { switch_windows, kba_switch_windows, ARG_NOT_USED , false},
+ { show_file_info, kba_show_file_info, ARG_NOT_USED , false},
+ { load_playlist, kba_load_playlist, ARG_NOT_USED , false},
+ { media_play, kba_play, ARG_NOT_USED , false},
+ { media_pause, kba_pause, ARG_NOT_USED , false},
+ { media_stop, kba_stop, ARG_NOT_USED , false},
+ { media_next, kba_play_next, ARG_NOT_USED , false},
+ { media_prev, kba_play_prev, ARG_NOT_USED , false},
+ { seek_forward_seconds, kba_seek, { .direction = FORWARDS, .scale =
SECONDS, .num = 10 }, false},
+ { seek_backward_seconds, kba_seek, { .direction = BACKWARDS, .scale =
SECONDS, .num = 10 }, false},
+ { seek_forward_minutes, kba_seek, { .direction = FORWARDS, .scale =
MINUTES, .num = 1 }, false},
+ { seek_backward_minutes, kba_seek, { .direction = BACKWARDS, .scale =
MINUTES, .num = 1 }, false},
+ { toggle, kba_toggle, { .scale = NUMBER, .num = 'G' },
false}
};
const size_t KeyActionHandlersSize = sizeof(KeyActionHandlers) /
sizeof(KeyActionHandler);
@@ -139,6 +142,7 @@
#define MY_K_TAB 9
#define MY_K_ENTER 13
+#define MY_K_ESCAPE 27
#define kb_CONTROL(x) ((x) - 'a' + 1)
const KeyBinding DefaultKeyBindings[] = {
@@ -174,6 +178,8 @@
{ '?', search_backward },
{ 'n', find_next_forward },
{ 'N', find_next_backward },
+ { 'v', visual },
+ { 'V', visual },
{ 'd', cut },
{ 'y', yank },
{ 'p', paste_after },
@@ -393,6 +399,12 @@
/* Is the key bound? */
found = false;
action = -1;
+ /* visual mode and ESC pressed ?*/
+ if (visual_mode_start != -1 && k == MY_K_ESCAPE){
+ visual_mode_start = -1;
+ return true;
+ }
+
for (i = 0; i < KeyBindingsSize; i++) {
if (KeyBindings[i].key == k) {
action = KeyBindings[i].action;
@@ -406,8 +418,10 @@
/* Execute theaction handler. */
for (i = 0; i < KeyActionHandlersSize; i++) {
if (KeyActionHandlers[i].action == action) {
- ((KeyActionHandlers[i].handler)(KeyActionHandlers[i].args));
- return true;
+ if (visual_mode_start == -1 || KeyActionHandlers[i].visual) {
+ ((KeyActionHandlers[i].handler)(KeyActionHandlers[i].args));
+ return true;
+ }
}
}
@@ -873,6 +887,23 @@
paint_error("Pattern not found: %s", mi_query_getraw());
}
+
+void
+kba_visual(KbaArgs a UNUSED)
+{
+ if (ui.active == ui.library) {
+ paint_message("No visual mode in library window. Sorry.");
+ return;
+ }
+
+ if (visual_mode_start == -1)
+ visual_mode_start = ui.active->voffset + ui.active->crow;
+ else
+ visual_mode_start = -1;
+
+ paint_playlist();
+}
+
/*
* TODO so much of the logic in cut() is similar with that of yank() above.
* combine the two, use an Args parameter to determine if the operation is
@@ -889,37 +920,49 @@
int input;
int n;
- /* determine range */
- n = 1;
- if (gnum_get() > 0) {
- n = gnum_get();
- gnum_clear();
- }
-
- /* get range */
- got_target = false;
- start = 0;
- end = 0;
- while ((input = getch()) && !got_target) {
- if (input == ERR)
- continue;
-
- switch (input) {
- case 'd': /* delete next n lines */
- start = ui.active->voffset + ui.active->crow;
- end = start + n;
- got_target = true;
- break;
-
- case 'G': /* delete to end of current playlist */
- start = ui.active->voffset + ui.active->crow;
- end = ui.active->nrows;
- got_target = true;
- break;
-
- default:
- ungetch(input);
- return;
+ if (visual_mode_start != -1) {
+ start = visual_mode_start;
+ end = ui.active->voffset + ui.active->crow;
+ visual_mode_start = -1;
+ if (start > end) {
+ int tmp = end;
+ end = start;
+ start = tmp;
+ }
+ end++;
+ } else {
+ /* determine range */
+ n = 1;
+ if (gnum_get() > 0) {
+ n = gnum_get();
+ gnum_clear();
+ }
+
+ /* get range */
+ got_target = false;
+ start = 0;
+ end = 0;
+ while ((input = getch()) && !got_target) {
+ if (input == ERR)
+ continue;
+
+ switch (input) {
+ case 'd': /* delete next n lines */
+ start = ui.active->voffset + ui.active->crow;
+ end = start + n;
+ got_target = true;
+ break;
+
+ case 'G': /* delete to end of current playlist */
+ start = ui.active->voffset + ui.active->crow;
+ end = ui.active->nrows;
+ got_target = true;
+ break;
+
+ default:
+ ungetch(input);
+ return;
+ }
}
}
@@ -1030,43 +1073,54 @@
return;
}
- /* determine range */
- n = 1;
- if (gnum_get() > 0) {
- n = gnum_get();
- gnum_clear();
- }
-
- /* get next input from user */
- got_target = false;
- start = 0;
- end = 0;
- while ((input = getch()) && !got_target) {
- if (input == ERR)
- continue;
-
- switch (input) {
- case 'y': /* yank next n lines */
- start = ui.active->voffset + ui.active->crow;
- end = start + n;
- got_target = true;
- break;
-
- case 'G': /* yank to end of current playlist */
- start = ui.active->voffset + ui.active->crow;
- end = ui.active->nrows;
- got_target = true;
- break;
-
- /*
- * TODO handle other directions ( j/k/H/L/M/^u/^d/ etc. )
- * here. this will be ... tricky.
- * might want to re-organize other stuff?
- */
-
- default:
- ungetch(input);
- return;
+ if (visual_mode_start != -1) {
+ start = visual_mode_start;
+ end = ui.active->voffset + ui.active->crow;
+ visual_mode_start = -1;
+ if (start > end) {
+ int tmp = end;
+ end = start;
+ start = tmp;
+ }
+ end++;
+ } else {
+ /* determine range */
+ n = 1;
+ if (gnum_get() > 0) {
+ n = gnum_get();
+ gnum_clear();
+ }
+ /* get next input from user */
+ got_target = false;
+ start = 0;
+ end = 0;
+ while ((input = getch()) && !got_target) {
+ if (input == ERR)
+ continue;
+
+ switch (input) {
+ case 'y': /* yank next n lines */
+ start = ui.active->voffset + ui.active->crow;
+ end = start + n;
+ got_target = true;
+ break;
+
+ case 'G': /* yank to end of current playlist */
+ start = ui.active->voffset + ui.active->crow;
+ end = ui.active->nrows;
+ got_target = true;
+ break;
+
+ /*
+ * TODO handle other directions ( j/k/H/L/M/^u/^d/ etc. )
+ * here. this will be ... tricky.
+ * might want to re-organize other stuff?
+ */
+
+ default:
+ ungetch(input);
+ return;
+ }
}
}
@@ -1079,6 +1133,7 @@
for (n = start; n < end; n++)
ybuffer_add(viewing_playlist->files[n]);
+ paint_playlist();
/* notify user # of rows yanked */
paint_message("Yanked %d files.", end - start);
}
@@ -1290,15 +1345,18 @@
{
if (ui.active == ui.library) {
ui.active = ui.playlist;
- if (ui.lhide)
+ if (ui.lhide) {
ui_hide_library();
+ paint_all();
+ }
} else {
ui.active = ui.library;
- if (ui.lhide)
+ if (ui.lhide) {
ui_unhide_library();
+ paint_all();
+ }
}
- paint_all();
paint_status_bar();
}
diff -r 3418951e8398 keybindings.h
--- a/keybindings.h Thu Feb 10 16:46:22 2011 -0500
+++ b/keybindings.h Fri Feb 11 18:26:29 2011 +0100
@@ -56,6 +56,7 @@
find_next_forward,
find_next_backward,
/* copy/cut/paste & undo/redo */
+ visual,
cut,
yank,
paste_after,
@@ -126,6 +127,7 @@
void kba_search(KbaArgs a);
void kba_search_find(KbaArgs a);
+void kba_visual(KbaArgs a);
void kba_cut(KbaArgs a);
void kba_yank(KbaArgs a);
void kba_paste(KbaArgs a);
diff -r 3418951e8398 paint.c
--- a/paint.c Thu Feb 10 16:46:22 2011 -0500
+++ b/paint.c Fri Feb 11 18:26:29 2011 +0100
@@ -271,6 +271,7 @@
{
playlist *plist;
bool hasinfo;
+ bool visual;
char *str;
int findex, row, col, colwidth;
int xoff, hoff, strhoff;
@@ -287,13 +288,22 @@
/* get index of file to show */
findex = row + ui.playlist->voffset;
+ /* determine if visual mode row */
+ visual = false;
+ if (visual_mode_start != -1 && ui.active == ui.playlist) {
+ if (visual_mode_start <= findex && findex <= ui.active->voffset +
ui.active->crow)
+ visual = true;
+ if (ui.active->voffset + ui.active->crow <= findex && findex <=
visual_mode_start)
+ visual = true;
+ }
+
/* apply row attributes */
wattron(ui.playlist->cwin, COLOR_PAIR(colors.playlist));
if (plist == playing_playlist && findex == player.qidx)
wattron(ui.playlist->cwin, COLOR_PAIR(colors.playing_playlist));
- if (row == ui.playlist->crow && ui.active == ui.playlist)
+ if ((row == ui.playlist->crow && ui.active == ui.playlist) || visual)
wattron(ui.playlist->cwin, A_REVERSE);
if (findex >= plist->nfiles)
@@ -392,7 +402,7 @@
if (findex >= plist->nfiles)
wattroff(ui.playlist->cwin, COLOR_PAIR(colors.tildas_playlist));
- if (row == ui.playlist->crow && ui.active == ui.playlist)
+ if ((row == ui.playlist->crow && ui.active == ui.playlist) || visual)
wattroff(ui.playlist->cwin, A_REVERSE);
if (plist == playing_playlist && findex == player.qidx)
diff -r 3418951e8398 vitunes.c
--- a/vitunes.c Thu Feb 10 16:46:22 2011 -0500
+++ b/vitunes.c Fri Feb 11 18:26:29 2011 +0100
@@ -26,6 +26,9 @@
playlist *viewing_playlist;
playlist *playing_playlist;
+/* visual mode start position */
+int visual_mode_start = -1;
+
/* signal flags */
volatile sig_atomic_t VSIG_QUIT = 0; /* 1 = quit vitunes */
volatile sig_atomic_t VSIG_RESIZE = 0; /* 1 = resize display */
diff -r 3418951e8398 vitunes.h
--- a/vitunes.h Thu Feb 10 16:46:22 2011 -0500
+++ b/vitunes.h Fri Feb 11 18:26:29 2011 +0100
@@ -44,6 +44,7 @@
/* record keeping */
extern playlist *viewing_playlist;
extern playlist *playing_playlist;
+extern int visual_mode_start;
/* signal flags referenced elsewhere */
extern volatile sig_atomic_t VSIG_QUIT;
Other related posts:
