Re: [hackers] [dmenu][PATCH] Vim-like vertical and horizontal keybindings
> From: Spenser Truex
Hello Spenser,
Thanks for the patch, a few comments :
Throughout the patch, there are indentation issues.
> Ensure that the keybindings are like vim, and not just made up.
This should be rephrased, precising that it's about vertical
mode, and nothing is made up.
> Old behaviour:
> Alt-h,l moves one element
> Alt-j,k moves one page
>
> New behaviour:
> Alt-h,l moves left/right or pages on a vertical listing
> Alt-j,k moves up/down on a vertical listing or pages a horizontal one
>
> This new behaviour maintains the heuristics for these keys:
> h is LEFT
> l is RIGHT
> j is DOWN
> k is UP
> ---
> config.def.h | 5 -
> dmenu.c | 37 +++--
> 2 files changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/config.def.h b/config.def.h
> index 1edb647..0d7748d 100644
> --- a/config.def.h
> +++ b/config.def.h
> @@ -13,8 +13,11 @@ static const char *colors[SchemeLast][2] = {
> [SchemeSel] = { "#ee", "#005577" },
> [SchemeOut] = { "#00", "#00" },
> };
> -/* -l option; if nonzero, dmenu uses vertical list with given number of
> lines */
> +/* -l option sets both of these to true */
> +/* if nonzero, dmenu uses given number of lines */
> static unsigned int lines = 0;
> +/* If nonzero, show vertical instead of a horizontal listing */
> +static unsigned int vertical = 0;
Is it really useful to have an option introducing different behaviour?
You could just keep the lines > 0 check.
> /*
> * Characters not considered part of a word while deleting words
> diff --git a/dmenu.c b/dmenu.c
> index 65f25ce..cb28913 100644
> --- a/dmenu.c
> +++ b/dmenu.c
> @@ -76,16 +76,16 @@ calcoffsets(void)
> {
> int i, n;
>
> - if (lines > 0)
> + if (vertical)
> n = lines * bh;
> else
> n = mw - (promptw + inputw + TEXTW("<") + TEXTW(">"));
> /* calculate which items will begin the next page and previous page */
> for (i = 0, next = curr; next; next = next->right)
> - if ((i += (lines > 0) ? bh : MIN(TEXTW(next->text), n)) > n)
> + if ((i += vertical ? bh : MIN(TEXTW(next->text), n)) > n)
> break;
> for (i = 0, prev = curr; prev && prev->left; prev = prev->left)
> - if ((i += (lines > 0) ? bh : MIN(TEXTW(prev->left->text), n)) >
> n)
> + if ((i += vertical ? bh : MIN(TEXTW(prev->left->text), n)) > n)
> break;
> }
>
> @@ -141,7 +141,7 @@ drawmenu(void)
> x = drw_text(drw, x, 0, promptw, bh, lrpad / 2, prompt, 0);
> }
> /* draw input field */
> - w = (lines > 0 || !matches) ? mw - x : inputw;
> + w = (vertical || !matches) ? mw - x : inputw;
> drw_setscheme(drw, scheme[SchemeNorm]);
> drw_text(drw, x, 0, w, bh, lrpad / 2, text, 0);
>
> @@ -151,7 +151,7 @@ drawmenu(void)
> drw_rect(drw, x + curpos, 2, 2, bh - 4, 1, 0);
> }
>
> - if (lines > 0) {
> + if (vertical) {
> /* draw vertical list */
> for (item = curr; item != next; item = item->right)
> drawitem(item, x, y += bh, mw - x);
> @@ -384,10 +384,18 @@ keypress(XKeyEvent *ev)
> goto draw;
> case XK_g: ksym = XK_Home; break;
> case XK_G: ksym = XK_End; break;
> - case XK_h: ksym = XK_Up;break;
> - case XK_j: ksym = XK_Next; break;
> - case XK_k: ksym = XK_Prior; break;
> - case XK_l: ksym = XK_Down; break;
> + case XK_j:
> +vertical ? ksym = XK_Down : (ksym = XK_Next);
I think it would better to keep something like:
case XK_j: ksym = vertical ? XK_Down : XK_Next; break;
With eventually the break on a new line.
Same for those other ones.
> +break;
> + case XK_k:
> +vertical ? ksym = XK_Up : (ksym = XK_Prior);
> +break;
> +case XK_h:
> +vertical ? ksym = XK_Prior : (ksym = XK_Up);
> +break;
> + case XK_l:
> +vertical ? ksym = XK_Next : (ksym = XK_Down);
> +break;
> default:
> return;
> }
> @@ -437,11 +445,11 @@ insert:
> calcoffsets();
> break;
> case XK_Left:
> - if (cursor > 0 && (!sel || !sel->left || lines > 0)) {
> + if (cursor > 0 && (!sel || !sel->left || vertical)) {
> cursor = nextrune(-1);
> break;
> }
> - if (lines > 0)
> + if (vertical)
> return;
> /* fallthrough */
> case XK_Up:
> @@ -477,7 +485,7 @@ insert:
> cursor = nextrune(+1);
> break;
> }
> - if (lines > 0)
> + if (vertical)
>
[hackers] [dmenu][PATCH] Vim-like vertical and horizontal keybindings
From: Spenser Truex
Ensure that the keybindings are like vim, and not just made up.
Old behaviour:
Alt-h,l moves one element
Alt-j,k moves one page
New behaviour:
Alt-h,l moves left/right or pages on a vertical listing
Alt-j,k moves up/down on a vertical listing or pages a horizontal one
This new behaviour maintains the heuristics for these keys:
h is LEFT
l is RIGHT
j is DOWN
k is UP
---
config.def.h | 5 -
dmenu.c | 37 +++--
2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/config.def.h b/config.def.h
index 1edb647..0d7748d 100644
--- a/config.def.h
+++ b/config.def.h
@@ -13,8 +13,11 @@ static const char *colors[SchemeLast][2] = {
[SchemeSel] = { "#ee", "#005577" },
[SchemeOut] = { "#00", "#00" },
};
-/* -l option; if nonzero, dmenu uses vertical list with given number of lines
*/
+/* -l option sets both of these to true */
+/* if nonzero, dmenu uses given number of lines */
static unsigned int lines = 0;
+/* If nonzero, show vertical instead of a horizontal listing */
+static unsigned int vertical = 0;
/*
* Characters not considered part of a word while deleting words
diff --git a/dmenu.c b/dmenu.c
index 65f25ce..cb28913 100644
--- a/dmenu.c
+++ b/dmenu.c
@@ -76,16 +76,16 @@ calcoffsets(void)
{
int i, n;
- if (lines > 0)
+ if (vertical)
n = lines * bh;
else
n = mw - (promptw + inputw + TEXTW("<") + TEXTW(">"));
/* calculate which items will begin the next page and previous page */
for (i = 0, next = curr; next; next = next->right)
- if ((i += (lines > 0) ? bh : MIN(TEXTW(next->text), n)) > n)
+ if ((i += vertical ? bh : MIN(TEXTW(next->text), n)) > n)
break;
for (i = 0, prev = curr; prev && prev->left; prev = prev->left)
- if ((i += (lines > 0) ? bh : MIN(TEXTW(prev->left->text), n)) >
n)
+ if ((i += vertical ? bh : MIN(TEXTW(prev->left->text), n)) > n)
break;
}
@@ -141,7 +141,7 @@ drawmenu(void)
x = drw_text(drw, x, 0, promptw, bh, lrpad / 2, prompt, 0);
}
/* draw input field */
- w = (lines > 0 || !matches) ? mw - x : inputw;
+ w = (vertical || !matches) ? mw - x : inputw;
drw_setscheme(drw, scheme[SchemeNorm]);
drw_text(drw, x, 0, w, bh, lrpad / 2, text, 0);
@@ -151,7 +151,7 @@ drawmenu(void)
drw_rect(drw, x + curpos, 2, 2, bh - 4, 1, 0);
}
- if (lines > 0) {
+ if (vertical) {
/* draw vertical list */
for (item = curr; item != next; item = item->right)
drawitem(item, x, y += bh, mw - x);
@@ -384,10 +384,18 @@ keypress(XKeyEvent *ev)
goto draw;
case XK_g: ksym = XK_Home; break;
case XK_G: ksym = XK_End; break;
- case XK_h: ksym = XK_Up;break;
- case XK_j: ksym = XK_Next; break;
- case XK_k: ksym = XK_Prior; break;
- case XK_l: ksym = XK_Down; break;
+ case XK_j:
+vertical ? ksym = XK_Down : (ksym = XK_Next);
+break;
+ case XK_k:
+vertical ? ksym = XK_Up : (ksym = XK_Prior);
+break;
+case XK_h:
+vertical ? ksym = XK_Prior : (ksym = XK_Up);
+break;
+ case XK_l:
+vertical ? ksym = XK_Next : (ksym = XK_Down);
+break;
default:
return;
}
@@ -437,11 +445,11 @@ insert:
calcoffsets();
break;
case XK_Left:
- if (cursor > 0 && (!sel || !sel->left || lines > 0)) {
+ if (cursor > 0 && (!sel || !sel->left || vertical)) {
cursor = nextrune(-1);
break;
}
- if (lines > 0)
+ if (vertical)
return;
/* fallthrough */
case XK_Up:
@@ -477,7 +485,7 @@ insert:
cursor = nextrune(+1);
break;
}
- if (lines > 0)
+ if (vertical)
return;
/* fallthrough */
case XK_Down:
@@ -715,9 +723,10 @@ main(int argc, char *argv[])
} else if (i + 1 == argc)
usage();
/* these options take one argument */
- else if (!strcmp(argv[i], "-l")) /* number of lines in
vertical list */
+ else if (!strcmp(argv[i], "-l")){ /* number of lines in
vertical list */
lines = atoi(argv[++i]);
- else if (!strcmp(argv[i], "-m"))
+vertical = 1;
+} else if (!strcmp(argv[i], "-m"))
mon =
