ReimuNotMoe / ydotool

Generic command-line automation tool (no X!)
GNU Affero General Public License v3.0
1.52k stars 81 forks source link

No Options possible #245

Open Dr0med4r opened 5 months ago

Dr0med4r commented 5 months ago

There is a bug introduced by #238 (Client/ydotool.c#L137) that when you use ydotool type -f file.txt there is a Not a valid option error. The problem is, that it is checked if the option is help or version else exit. But now you can't use other options in subcommands.

LKS90 commented 4 months ago

Yeah, can't do anything with mousemove, no matter what I try:

host@device:~/ydotool/build$ ./ydotool mousemove -x 100 -y 100
./ydotool: invalid option -- 'x'
Not a valid option

Usage: ydotool [...]

The other options also don't work:

./ydotool mousemove
Usage: mousemove [OPTION]... [-x <xpos> -y <ypos>] [-- <xpos> <ypos>]
Move mouse pointer or wheel.

Options:
  -w, --wheel                Move mouse wheel relatively
  -a, --absolute             Use absolute position, not applicable to wheel
  -x, --xpos                 X position
  -y, --ypos                 Y position
  -h, --help                 Display this help and exit

You need to disable mouse speed acceleration for correct absolute movement.

let's try the absolute option:

./ydotool mousemove -a 100 100
./ydotool: invalid option -- 'a'
Not a valid option

Usage: ydotool [OPTION] <cmd> <args>
Options:
  -h, --help                 Display this help and exit
  -V, --version              Show version information
Available commands:
  click
  mousemove
Enkidu-Aururu commented 4 months ago

I see the same problem "invalid option" everywhere when using build from present master

$ ydotool -V ydotool version(or hash): v1.0.4-32-gac76271

Version before https://github.com/ReimuNotMoe/ydotool/pull/238 works.

aeleos commented 3 months ago

I compiled the master branch and had the same issue. Switching to commit e573cfb3aa94f92a78b5d6bc669026c4119e31eb fixed this.

jonas73x commented 3 months ago

I reverted back to the "old" way of doing it (but adding an option for version) and it seems to work fine. My coding skills are poor and I'm sure there are valid reasons to use the elegant getopt_long method, but for me it created more headache than it solved. So if this can be verified to work for others, perhaps someone can make the fix so that master works again? In more detaild what I did was:

remove:

static struct option long_options[] = {
    {"help", no_argument, 0, 'h'},
    {"version", no_argument, 0, 'V'},
};

int opt = getopt_long(argc, argv, "hV", long_options, NULL);
if (opt != -1)
{
    switch (opt) {
        case 'h':
            show_help();
            exit(0);

        case 'V':
            show_version();
            exit(0);

        default:
            puts("Not a valid option\n");
            show_help();
            exit(1);
    }
}

and then add back:

    if (argc < 2 || strncmp(argv[1], "-h", 2) == 0 || strncmp(argv[1], "--h", 3) == 0 || strcmp(argv[1], "help") == 0) {
            show_help();
            return 0;
    }
    else if (strncmp(argv[1], "-V", 2) == 0 || strncmp(argv[1], "--V", 3) == 0 || strcmp(argv[1], "version") == 0) {
            show_version();
            return 0;
    }
Paiusco commented 1 month ago

Pretty much sorry for that and for disappearing for so long. Hopefully now my smaller PR #252 cleans a bit more without making it impossible to use.

We should consider adding some unit tests under here, to make it simpler to make sure basic functionality works.