behave-screen-edge code is a mess (cmd_behave_screen_edge) about xdotool HOT 15 OPEN

I'll buy the 'code is a mess' argument given , but I'm curious - what problem are you encountering? I'm not sure how best to answer your question without knowing your problem, so before I answer any code linting/analysis questions, let's figure out what that is! :)

from xdotool.

To be honest, having reviewed this code briefly just now, it looks like I refactored hastily this code a few times (which explains the end-of-loop 'dead assignments' and such), but again, let's get to what your problem is so that we may solve it :)

from xdotool.

I was having issues with xdotool type --window $ID somekeystore, it would be entered in the current window instead of the window selected by $ID (actually, I tried searchwindow --name Firefox).

Our of curiousity, I ran scan-build on this project which found over 30 potentional bugs. I am in process of fixing them, you can expect a pull request tomorrow for that.

from xdotool.

I rely mostly on the test suite to find bugs; any chance you can add tests for any bugs you're fixing that are indicated by your tools?

from xdotool.

I will try to add tests where applicable. Some of the bugs occur when feeding unexpected input, or rely on undefined behavior.

Aside, what do you think of using the goto keyword?

This:

  void *p1 = NULL;
  void *p2 = NULL;

  if (cond) {
    return EXIT_FAILURE;
  }

  p1 = malloc(...);

  if (condition) {
    /* note: leak of p1 */
    return EXIT_FAILURE;
  }

...

  free(p1);
  free(p2);

  return EXIT_SUCCESS;

could be converted to:

  int ret = EXIT_FAILURE;
  void *p1 = NULL;
  void *p2 = NULL;

  if (cond) {
    return EXIT_FAILURE;
  }

  p1 = malloc(...);

  if (condition) {
    goto free_p1;
  }

...

  ret = EXIT_SUCCESS;
free_p2:
  free(p2);
free_p1:
  free(p1);
  return ret;

(real example: cmd_exec in cmd_exec.c, p1 is terminator and p2 is command)

Btw, how to test for crashes? Undefined behavior is difficult to test...

from xdotool.

Regarding typing to firefox; I just tested again now with Firefox 25 it seems to work:

% xdotool getactivewindow 
37748739
% xdotool search --classname Navigator 
56623242
% xdotool search --classname Navigator   key ctrl+t
# result: firefox opens a new tab

from xdotool.

re: using goto for cleanup like like a finally(java)/ensure(ruby)/defer(go) +1 I would approve this refactoring.

from xdotool.

key is working, but type isn't:

xdotool search --name behave type x

I have pushed the fixes I have done so far here:
https://github.com/Lekensteyn/xdotool/compare/clang-fixes

There are still 18 memleaks, 1 dead assignment and 11 logic errors (garbage/undefined) to fix (including the ones from this issue).

from xdotool.

it'd be nice if there were a more straight forward way to add cleanup tasks though, instead of having multiple goto labels, just queue up some function calls (like 'defer' in go) and call them all in order at the end.

Interested in implementing that? Shouldn't be too hard ;)

from xdotool.

For just freeing up resources one label is enough (free(NULL)) is fine, that should not need special treatment.

from xdotool.

key is working, but type isn't

'type' defaults to --window 0, I believe, incorrectly. This works:

% xdotool search --classname Navigator   type --window "%1" 'hello world'

cmd_key.c has:

    /* use %1 if there is a window stack */
   if (window_arg == NULL && context->nwindows > 0) {
     window_arg = "%1";
   }

And cmd_type does not. cmd_type should also do this "default to %1 if window_arg is not specified and there is a window stack"

from xdotool.

Also, I very much appreciate your energy put into helping improve this project :)

from xdotool.

Thanks for the tool, it already saved me some time as a workaround for bugs :-)

What do you think of the pattern I used here:
Lekensteyn@38cd31b

I could also have introduced some macros, but I feel that this is simple enough to avoid macros.

from xdotool.

38cd31b looks good to me!

from xdotool.

Agreed re: macros; it's simple enough.

from xdotool.