GNOME Bugzilla – Bug 514447
patch for child setup function
Last modified: 2010-03-17 19:24:06 UTC
Hi, the attached patch allows one to pass in a child setup function, which is executed just before the exec() from vte_terminal_fork_command. I named the new function vte_terminal_fork_command_with_setup. The resulting changes were mostly adding the requisite two parameters to a parade of internal functions. I was somewhat tempted to rewrite everything to be based on a "structure with flags and padding" approach to allow adding even more options later without requiring us to add yet another _fork_command_* function, but that would have been pretty invasive. The current version already allows you to specify almost everything I can think of that most users would want, and this new child setup function should pretty much fill in the rest. Why do I need this patch? For the Hotwire hypershell, I wanted the ability to spawn a terminal but having its stdin redirected from a pipe. So say you do: cat foo | less, The "cat foo" runs in the Hotwire process as normal (i.e. not /bin/cat), but "less" expands to "term less", with its standard input redirected from a pipe.
Created attachment 104453 [details] [review] patch to allow a child setup function
Sounds useful. Even better would be to do a parallel of g_spawn_*(), or even better, make vte more usable with g_spawn_*(). No idea what exactly that means.
I don't see a lot of value in putting VTE actually under g_spawn somehow, because to do almost anything interesting you need the Terminal widget. Well, actually it might make sense to put the pseudoterminal allocation inside GLib since it is useful in specialized circumstances, but it's a bit outside the scope of what I want to try to tackle at the moment. Now, having a g_spawn like API for vte's fork_command might be a good idea; what I was talking about with just passing in a structure with flags is even more extensible since you could pass data. I was going to start rewriting it for that, but I had a hard time thinking of something you'd want to add that isn't already there, and that you couldn't do in the setup function. Do you know offhand of any other open bugs for API additions in this area?
I wasn't suggesting putting VTE under g_spawn, but instead making it useful with g_spawn. Like, you do g_spawn and connect a VteTerminal to it (like using vte_terminal_set_pty()). For that you need the pty creation as you said. That can stay in vte, so one would do, roughly speaking: master, slave = vte_pty_new(); g_spawn_*(... slave ...); vte_terminal_set_pty (master); Does that work? I just want to be able to provide the whole functionality of g_spawn (and without duplicating it). A vte function taking all the same args as g_spawn works too...
Created attachment 104483 [details] [review] make child setup function public I see what you're saying. So the pty code has two major parts; the PTY creation and setting up the child process just right. What you're saying is people who want to do something special can already create a PTY manually, and then set up the child process on their own. Looking more closely, the code I wanted to avoid duplicating is vte_pty_child_setup. If we made that function public, then I could just do the fork/exec using Python's subprocess (and GLib users could use g_spawn), but calling it in the child. So...something like this?
Yeah, that's more like what I meant. Not sure how useful that is. Does it do the job for you? Chris, Mariano, what do you guys think?
I attached a patch to bug 320128 that includes a variant of vte_terminal_fork_command to pass child setup function. (The patch here has some useful extra bits in it in the pythin binding and the demo app.)
This is now fixed on master with the new vte_terminal_fork_command_full convenience API. Wrapping the new API for python is bug 613180.