Add Programname and Windowname to logs #152

t0mOURCSSn · 2016-07-28T18:14:24Z

@kernc
Hi I implemented a feature to include the program & window name in logkeys' logs.
I would be honored to have my code in your project.
New to git and the whole GitHub forking aspect.
I'll gladly explain in detail the differences.
Attached you will find an example log.

The main changes are:
-added argument "--programinfo"
-the code "on key press (EV_MAKE)" was split into 2 functions, one for newlines and one to encode the scan_codes

Issues/Questions:
Currently using "xprop" to determine the window id and name, maybe there is a better command?

…sages in main while loop

Mission: Add possibility to include Program and Window name in logs Added argument "--programinfo" The code "on key press (EV_MAKE)" was split into 2 functions, one for newlines and one to encode the scan_codes Added check for program change, which triggers a newline Commented old function that required X11 Cleaned up code On branch windowname-no-x11 Changes to be committed: modified: src/args.cc modified: src/logkeys.cc Untracked files: keymap.map test.log

kernc · 2016-08-02T13:14:07Z

Thanks for this, I'd be honored to accept it. It fulfills one of the first enhancement requests, #3.

kernc · 2016-08-02T13:18:08Z

src/logkeys.cc

+    //// on processid change update program_info write '[process name] "process title" > '
+    //// on process title change (like firefox tabs) would be better. possibly more ressource intensive?
+    if (args.flags & FLAG_PROGRAMINFO) {
+      window_id = execute(COMMAND_STR_AWID);


This spawns three new processes on each keypress. This indeed feels somewhat intensive. Did you perhaps investigate how cumbersome the use of relevant Xlib function calls would be?

kernc · 2016-08-02T14:06:08Z

I think there is a user's manual file somewhere in the distribution. Perhaps mention the new switch there as well?

Overall, looks good to me. @jzohrab, do you, as the previous most active maintainer, have something to add?

kernc · 2016-08-02T14:07:14Z

src/logkeys.cc

+
+// active window id, title, name
+#define COMMAND_STR_AWID    "xprop -root 32x '\\t$0' _NET_ACTIVE_WINDOW | cut -f 2"
+#define COMMAND_STR_AWTITLE "xprop -id $(" COMMAND_STR_AWID ") _NET_WM_NAME | cut -d '=' -f 2"


If the window title contains '=', this will include only the portion of the tile up to it. Use cut -d'=' -f2-.

t0mOURCSSn · 2016-08-03T20:32:17Z

Hi
you raise valid points. I will look into them as soon as i have the time, that should be before the end of the week.

t0mOURCSSn · 2016-08-06T17:49:53Z

src/logkeys.cc

+      window_id = execute(COMMAND_STR_AWID);
+
+      if (window_id.compare(old_window_id) != 0) {
+        cur_process_name = execute(COMMAND_STR_AWPNAME);


COMMAND_STR_AWPNAME contains COMMAND_STR_AWID to determine the window id.
Since here we already have the window id, it would be better to change COMMAND_STR_AWPNAME and COMMAND_STR_AWTITLE to use the already determined window_id, maybe by splitting them up?
this way we would use xprop to get the window id only once, instead of 3 times.

t0mOURCSSn · 2016-08-06T18:31:30Z

src/logkeys.cc

    if (args.flags & FLAG_WINDOW_TITLE) {
      window_id = execute(COMMAND_STR_AWID);

+      //// really ugly!


really ugly this part!

kernc · 2016-08-06T19:32:28Z

Just to be safe, don't go into replacing xprop calls with Xlib calls. I now think it's better if logkeys doesn't depend on X.

t0mOURCSSn · 2016-08-06T19:35:34Z

I made the changes you suggested in the comments.

Apart from the 'really ugly' commented part, I think this might be the final non-X11 version. (unless we find other errors/improvements that is)
I am working on an X11 version with events.
You might want to consider adding a "milestone"/tag either before this current pull request or before the X11 version. That way we could allow for backwards-compatibility or let people download the "OG"-version.

For the X11 version:
I tried to implement X11 with XNextEvent to recieve active-window-changed events with XSelectInput(Display, Window, PropertyChangeMask) and get WM_NAME and WM_CLASS with the help of XGetWindowProperty().

While I was able to make the code snippet work in a separate while loop in main(), I was unable to make it work in the default while (read(input_fd, &event, sizeof(struct input_event)) > 0) loop.
Both XNextEvent and your while loop are listening for events.
Possible solutions I found:

Find an alternative to XNextEvent. There are ways to cicle for events in X11 but I can't seem to get further with X11 without expending unreasonable efforts.
Asynchronously multithread the X11 get-window-name function and the main while loop. The function would just bruteforce a newline with the necessary information and the main loop remains untouched. It's currently working well but I still have to figure out some minor things.

I just saw your update. How do we proceed? My code is almost finished, should we just keep it in a separate fork?

t0mOURCSSn · 2016-08-08T15:11:43Z

FYI: I did a quick google search and it seems saving clipboard content can only be done with the help of X11.

kernc · 2016-08-14T15:25:23Z

The problem with Xlib is that, unless it was dynamically loaded if present (dlopen & co.), it would have to be installed in order to compile / run logkeys. Which would make it somewhat less useful on headless servers where X is not used / running / desired / installed. If anything, X calls should be ifdef-ed with a compile-time flag.

Similarly to xprop, one can get clipboard contents with xclip (with X running, of course). But don't ram too much into this PR.

kernc · 2016-08-14T15:28:18Z

src/logkeys.cc

@@ -48,10 +48,17 @@
 #define COMMAND_STR_DUMPKEYS ( EXE_DUMPKEYS " -n | " EXE_GREP " '^\\([[:space:]]shift[[:space:]]\\)*\\([[:space:]]altgr[[:space:]]\\)*keycode'" )
 #define COMMAND_STR_GET_PID  ( (std::string(EXE_PS " ax | " EXE_GREP " '") + program_invocation_name + "' | " EXE_GREP " -v grep").c_str() )

+#define COMMAND_STR_DEVICE    EXE_GREP " -E 'Handlers|EV' /proc/bus/input/devices | " EXE_GREP " -B1 120013 | " EXE_GREP " -Eo event[0-9]+"


-B1 120013 is not equal to what was here before?

…ly interested in "visible" key presses. changed printf into snprintf. updated manual file to include --window-title. #defined commands that were left in code previously.

t0mOURCSSn · 2016-08-19T16:57:23Z

Is there still interest in this feature?
I have no idea on exactly how to use ifdef in order to find out if x is installed.

t0mOURCSSn and others added 7 commits July 25, 2016 18:28

copied modifications i made to an on older version to the current one

40d43bf

code cleaned up. ready to test.

7475d35

added improved commands for finding window names. changed logging mes…

982e23f

…sages in main while loop

usage info now includes voice "--programinfo"

dc7690b

fixed bug where window title got truncated

d5bec79

Update logkeys.cc

6126a01

kernc reviewed Aug 2, 2016
View reviewed changes

t0mOURCSSn added 3 commits August 6, 2016 19:13

changed --programinfo flag to --window-title

a037bc7

Update usage.cc

85c8f39

changed programinfo flag to window-title

cc9510a

t0mOURCSSn reviewed Aug 6, 2016
View reviewed changes

Changed the command strings for slight performance improvement

b2e44dc

t0mOURCSSn reviewed Aug 6, 2016
View reviewed changes

kernc reviewed Aug 14, 2016
View reviewed changes

moved window-info update code into EV_MAKE if clause, since we are on…

6268cc7

…ly interested in "visible" key presses. changed printf into snprintf. updated manual file to include --window-title. #defined commands that were left in code previously.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Programname and Windowname to logs #152

Add Programname and Windowname to logs #152

t0mOURCSSn commented Jul 28, 2016 •

edited

kernc commented Aug 2, 2016

kernc Aug 2, 2016

kernc commented Aug 2, 2016

kernc Aug 2, 2016 •

edited

t0mOURCSSn commented Aug 3, 2016

t0mOURCSSn Aug 6, 2016

t0mOURCSSn Aug 6, 2016

kernc commented Aug 6, 2016

t0mOURCSSn commented Aug 6, 2016 •

edited

t0mOURCSSn commented Aug 8, 2016

kernc commented Aug 14, 2016

kernc Aug 14, 2016

t0mOURCSSn commented Aug 19, 2016

Add Programname and Windowname to logs #152

Are you sure you want to change the base?

Add Programname and Windowname to logs #152

Conversation

t0mOURCSSn commented Jul 28, 2016 • edited

kernc commented Aug 2, 2016

kernc Aug 2, 2016

Choose a reason for hiding this comment

kernc commented Aug 2, 2016

kernc Aug 2, 2016 • edited

Choose a reason for hiding this comment

t0mOURCSSn commented Aug 3, 2016

t0mOURCSSn Aug 6, 2016

Choose a reason for hiding this comment

t0mOURCSSn Aug 6, 2016

Choose a reason for hiding this comment

kernc commented Aug 6, 2016

t0mOURCSSn commented Aug 6, 2016 • edited

t0mOURCSSn commented Aug 8, 2016

kernc commented Aug 14, 2016

kernc Aug 14, 2016

Choose a reason for hiding this comment

t0mOURCSSn commented Aug 19, 2016

t0mOURCSSn commented Jul 28, 2016 •

edited

kernc Aug 2, 2016 •

edited

t0mOURCSSn commented Aug 6, 2016 •

edited