Re: [hackers] [slstatus][PATCH keymap 1/2] Add keymap component

2018-05-22 Thread Aaron Marcher

Michael,

Thank you very much for your great work. However I have a few things for 
you to change.



-   components/backlight\



-/* backlight */
-const char *backlight_perc(const char *);
-


Why are you removing backlight from Makefile and slstatus.h?


+   for (i = 0; i < sizeof(EXCLUDES)/sizeof(EXCLUDES[0]); ++i)
+   if (strstr(sym, EXCLUDES[i]))
+   return 0;


According to coding style: "Use block for single statement ifs".


+/* Given a xkb_symbols string (syms) retrieve
+ * and populate the provided layout and variant
+ * strings
+ */


I think this specific comment is unnecessary as the function explains 
itself.



+   char *token,
+*copy,
+*delims = "+:";


I don't think that the newlines are necessary here, it can be put in one 
line. Additionally the indentation is wrong. And try to only declare 
variables at the beginning of a block and initialize them later.



+   if (IsLayoutOrVariant(token)
+   && !(strlen(token) == 1 && isdigit(token[0]))) {


Indentation is wrong here, tabs to the last indent and spaces for 
alignment afterwards.



+const char *
+keymap(void)
+{
+   static char layout[LAYOUT_MAX];
+   


Whitespace error here.


+   desc = XkbAllocKeyboard();
+   if (!desc) {


This can be put into one if clause, no need for two lines.

Additionally, can you send one patch instead of a series regarding your 
"Free strduped string"?


Cheers!
Aaron

--
Web: https://drkhsh.at/ or http://drkhsh5rv6pnahas.onion/
Gopher: gopher://drkhsh.at or gopher://drkhsh5rv6pnahas.onion
GPG: 0x7A65E38D55BE96FE
Fingerprint: 4688 907C 8720 3318 0D9F AFDE 7A65 E38D 55BE 96FE



[hackers] [slstatus][PATCH keymap 1/2] Add keymap component

2018-05-22 Thread michaelbuch12
From: Michael Buch 

Adding a new keymap component that will
indicate the current keyboard layout (language)
and variant if any was set. I use the
standard X11 XKB APIs to retrieve and parse
the xkb_symbols set with setxkbmap.
---
 Makefile|  2 +-
 components/keymap.c | 96 +
 config.def.h|  2 +
 slstatus.h  |  6 +--
 4 files changed, 102 insertions(+), 4 deletions(-)
 create mode 100644 components/keymap.c

diff --git a/Makefile b/Makefile
index 8e18969..c7b9d0c 100644
--- a/Makefile
+++ b/Makefile
@@ -6,7 +6,6 @@ include config.mk
 
 REQ = util
 COM =\
-   components/backlight\
components/battery\
components/cpu\
components/datetime\
@@ -15,6 +14,7 @@ COM =\
components/hostname\
components/ip\
components/kernel_release\
+   components/keymap\
components/keyboard_indicators\
components/load_avg\
components/netspeeds\
diff --git a/components/keymap.c b/components/keymap.c
new file mode 100644
index 000..67d605e
--- /dev/null
+++ b/components/keymap.c
@@ -0,0 +1,96 @@
+/* See LICENSE file for copyright and license details. */
+#include 
+#include 
+#include  
+#include 
+
+#include "../util.h"
+
+#define LAYOUT_MAX 256
+
+/* Given a token (sym) from the xkb_symbols string
+ * check whether it is a valid layout/variant. The
+ * EXCLUDES array contains invalid layouts/variants
+ * that are part of the xkb rules config.
+ */
+int
+IsLayoutOrVariant(char *sym)
+{
+   static const char* EXCLUDES[] = { "evdev", "inet", "pc", "base" };
+
+   size_t i;
+   for (i = 0; i < sizeof(EXCLUDES)/sizeof(EXCLUDES[0]); ++i)
+   if (strstr(sym, EXCLUDES[i]))
+   return 0;
+
+   return 1;
+}
+
+/* Given a xkb_symbols string (syms) retrieve
+ * and populate the provided layout and variant
+ * strings
+ */
+void
+GetKeyLayout(char *syms, char layout[], int groupNum)
+{
+   char *token,
+*copy,
+*delims = "+:";
+   int group = 0;
+
+   copy = strdup(syms);
+   token = strtok(copy, delims);
+   while (token != NULL && group <= groupNum) {
+   /* Ignore :2,:3,:4 which represent additional layout
+* groups
+*/
+   if (IsLayoutOrVariant(token)
+   && !(strlen(token) == 1 && isdigit(token[0]))) {
+   strncpy (layout, token, LAYOUT_MAX);
+   group++;
+   }
+
+   token = strtok(NULL,delims);
+   }
+}
+
+const char *
+keymap(void)
+{
+   static char layout[LAYOUT_MAX];
+   
+   Display *dpy;
+   char *symbols = NULL;
+   XkbDescRec* desc = NULL;
+
+   memset(layout, '\0', LAYOUT_MAX);
+
+   if (!(dpy = XOpenDisplay(NULL))) {
+   warn("XOpenDisplay: Failed to open display");
+   return NULL;
+   }
+
+   desc = XkbAllocKeyboard();
+   if (!desc) {
+   warn("XkbGetNames: failed to get XkbSymbolsNameMask keyboard 
component");
+   XCloseDisplay(dpy);
+   return NULL;
+   }
+
+   XkbGetNames(dpy, XkbSymbolsNameMask, desc);
+   if (desc->names) {
+   XkbStateRec state;
+   XkbGetState(dpy, XkbUseCoreKbd, &state);
+
+   symbols = XGetAtomName(dpy, desc->names->symbols);
+   GetKeyLayout(symbols, layout, state.group);
+   XFree(symbols);
+   } else {
+   warn("XkbGetNames: failed to retrieve symbols for keys");
+   return NULL;
+   }
+
+   XkbFreeKeyboard(desc, XkbSymbolsNameMask, 1);
+   XCloseDisplay(dpy);
+   return layout;
+}
diff --git a/config.def.h b/config.def.h
index 45320a3..33da6a4 100644
--- a/config.def.h
+++ b/config.def.h
@@ -34,6 +34,8 @@ static const char unknown_str[] = "n/a";
  * ipv6 IPv6 addressinterface name (eth0)
  * kernel_release   `uname -r`  NULL
  * keyboard_indicators  caps/num lock indicatorsNULL
+ * keymap   layout (variant) of current NULL
+ *  keyboard
  * load_avg load averageNULL
  * netspeed_rx  receive network speed   interface name (wlan0)
  * netspeed_tx  transfer network speed  interface name (wlan0)
diff --git a/slstatus.h b/slstatus.h
index a18b881..079a8db 100644
--- a/slstatus.h
+++ b/slstatus.h
@@ -1,8 +1,5 @@
 /* See LICENSE file for copyright and license details. */
 
-/* backlight */
-const char *backlight_perc(const char *);
-
 /* battery */
 const char *battery_perc(const char *);
 const char *battery_state(const char *);
@@ -37,6 +34,9 @@ const char *kernel_release(void);
 /* keyboard_indicators */
 const char *keyboard_indicators(void);
 
+/* keymap */
+const char *keymap(void);
+
 /* load_avg */
 const char *load_avg(const c