cole-h / agenix-cli

Companion tool to https://github.com/ryantm/agenix
Apache License 2.0
31 stars 3 forks source link

Failed to spawn editor #2

Closed ryantm closed 3 years ago

ryantm commented 3 years ago
$ /home/ryantm/p/agenix-rs/result/bin/agenix p-vpn.age
Error:
   0: Failed to spawn editor 'emacsclient -t'
   1: No such file or directory (os error 2)

Location:
   src/cli.rs:245

Looks like it doesn't like that my EDITOR env var has commandline arguments.

cole-h commented 3 years ago

I'm not sure the best way to solve this.

I could just split the EDITOR on whitespace and set everything after the first "group" as an argument, but what about the (extremely niche, probably never happens, and even less likely they use this project) case where somebody's editor binary is actually some cool editor (spaces and all)?

What I've seen suggested in other communities is making a wrapper script for this case -- e.g. pkgs.writeShellScriptBin "emt" ''${pkgs.emacs}/bin/emacsclient -t "$@"''.

To be honest, I don't have any qualms about breaking the "editor with space in filename" situation if you don't want to make a wrapper script, but it's easily avoided by just not having a space in your EDITOR in the first place ;)

ryantm commented 3 years ago

Probably I should make a wrapper script to work around issues like this, but I think it is a common enough recommendation that probably it should split on the string, or use some higher-level process execution API that takes a string.

https://www.emacswiki.org/emacs/EmacsClient for example recommends what I'm doing.

ryantm commented 3 years ago

I guess you could also try to see if EDITOR is a binary and if that fails, split it.

cole-h commented 3 years ago

Fixed in 90dc2ea5510842d70dde74027da1d81942e9e00c.

cole-h commented 3 years ago

FWIW, checking if the EDITOR is a binary would be somewhat more involved, because we'd have to resolve it with PATH and then check if it's an executable (if it resolves at all). I don't remember if there's an easy way to resolve it and get the path back, but I think it's fine to not do that for now.