clarkwang / sexpect

Expect for Shells
GNU General Public License v3.0
120 stars 16 forks source link

[v2.3.12] Add an option to pass a file descriptor to the "send" command #33

Closed vaygr closed 2 years ago

vaygr commented 2 years ago

Inspired by this so named pipes could be used.

Passing it to -file returns:

[ERROR] not a regular file: /dev/fd/63

Not sure if it can be combined or split into its own arg like -file-fd.

clarkwang commented 2 years ago

For now send -file only accepts a regular file. send sets a limit and at most 1024 bytes can be sent so it need to check the file size before reading. It cannot get the file size of a named pipe.

What's the benefit of using a named pipe in your use case?

vaygr commented 2 years ago

Same as in the link in the description. I want to be able to pass <(dmenu -P) as an argument.

clarkwang commented 2 years ago

OK. Let me see if I can remove the regular file check.

vaygr commented 2 years ago

Another useful (somewhat related) thing would be to allow specifying - for -file to accept data from stdin. In both cases (named pipe and stdin) the 1024b limit can still be preserved, discarding everything else beyond that.

clarkwang commented 2 years ago

May be like this:

The idea is to make the behavior more explicit rather than silently discarding data so users would not be surprised.

vaygr commented 2 years ago

Yeah, I think that's great.

clarkwang commented 2 years ago

It's now available on the "devel" branch. Will merge to "master" after the doc is updated and tests are added.

vaygr commented 2 years ago

That's impressive, thanks! Testing it now:

$ ./sexpect -s /tmp/sp.sock send -fd <(dmenu) -limit 32
[ERROR] invalid number: /dev/fd/63

Probably makes sense to accept paths as well, not just fd by number?

vaygr commented 2 years ago

When I'm trying to use stdio, I have to hit extra Enter to make dmenu pop up:

./sexpect -s /tmp/sp.sock send -file - -limit 32 | dmenu
clarkwang commented 2 years ago

That's impressive, thanks! Testing it now:

$ ./sexpect -s /tmp/sp.sock send -fd <(dmenu) -limit 32
[ERROR] invalid number: /dev/fd/63

Probably makes sense to accept paths as well, not just fd by number?

Shell's <(cmd) is not a FD. It's a pathname, as is /dev/fd/63 in the error message.

clarkwang commented 2 years ago

When I'm trying to use stdio, I have to hit extra Enter to make dmenu pop up:

./sexpect -s /tmp/sp.sock send -file - -limit 32 | dmenu

sexpect's send sub-command is not supposed to output any meaningful data. It makes little sense to pipe the output to another command. According to your previous comments, you may want this:

./sexpect -s /tmp/sp.sock send -file <(dmenu) -limit 32
# OR
./sexpect -s /tmp/sp.sock send -fd 0 -limit 32 < <(dmenu)
# OR 
dmenu | ./sexpect -s /tmp/sp.sock send -fd 0 -limit 32
vaygr commented 2 years ago

Shell's <(cmd) is not a FD. It's a pathname, as is /dev/fd/63 in the error message.

Yeah, this is what I'm saying. Maybe accept FD paths as well here?

vaygr commented 2 years ago

sexpect's send sub-command is not supposed to output any meaningful data. It makes little sense to pipe the output to another command. According to your previous comments, you may want this:

./sexpect -s /tmp/sp.sock send -file <(dmenu) -limit 32
# OR
./sexpect -s /tmp/sp.sock send -fd 0 -limit 32 < <(dmenu)
# OR 
dmenu | ./sexpect -s /tmp/sp.sock send -fd 0 -limit 32

Yeah, sorry, I mismatched the commands. This worked:

$ dmenu <&- | ./sexpect -s /tmp/sp.sock send -cr -file - -limit 32
vaygr commented 2 years ago

Shell's <(cmd) is not a FD. It's a pathname, as is /dev/fd/63 in the error message.

Yeah, this is what I'm saying. Maybe accept FD paths as well here?

Ignore this. Given that you removed regular file check, as you said in your previous comment -file works perfectly with a path:

$ ./sexpect -s /tmp/sp.sock send -file <(dmenu) -cr -limit 32
vaygr commented 2 years ago

Yep, see my previous comment. I think all is good now and works as expected designed.

clarkwang commented 2 years ago

Merged to master: https://github.com/clarkwang/sexpect/releases/tag/v2.3.12

vaygr commented 2 years ago

Thanks a lot! I also submitted an AUR package and will keep maintaining it. Hopefully that'll increase popularity of this useful tool.