After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 585841 - Need vte_terminal_fork_command variant that return a GError
Need vte_terminal_fork_command variant that return a GError
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks: 320128 vte1.0 514447 588871
 
 
Reported: 2009-06-15 11:52 UTC by Christian Persch
Modified: 2010-03-17 18:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (36.47 KB, patch)
2009-06-15 11:54 UTC, Christian Persch
none Details | Review
current patch (56.82 KB, patch)
2009-12-04 18:47 UTC, Christian Persch
none Details | Review
different API (108.51 KB, patch)
2009-12-16 18:51 UTC, Christian Persch
none Details | Review
updated patch (109.68 KB, patch)
2010-01-25 14:32 UTC, Christian Persch
none Details | Review

Description Christian Persch 2009-06-15 11:52:24 UTC
For g-t, I need a way to get a GError back when fork_command fails, so I can display a meaningful message to the user instead of the useless "There was an error creating the child process for this terminal".

Attached patch provides this; it also adds extra child setup (bug 514447).
It will also make implementing bug 320128 easier.

The 2 different flags arguments aren't ideal; I tought about extending GSpawnFlags (taking the highest bits for our flags), but that probably wouldn't work well for the generated enum types nor for the bindings / introspection.

Behdad: could you take a look to see if this patch is sane?
Comment 1 Christian Persch 2009-06-15 11:54:49 UTC
Created attachment 136625 [details] [review]
proposed patch

Another question I have is why fork_command always uses G_SPAWN_CHILD_INHERITS_STDIN; is that necessary for something in our impl? vteapp seems to work without it...
Comment 2 Christian Persch 2009-07-20 21:02:58 UTC
If there are no comments, I'm going to commit this patch this week.
Comment 3 Christian Persch 2009-12-04 18:47:15 UTC
Created attachment 149109 [details] [review]
current patch
Comment 4 Behdad Esfahbod 2009-12-10 02:14:20 UTC
Looks good.  The spawn() docs can use another review.  Feel free to commit (though, I appreciate if you wait till next week).
Comment 5 Christian Persch 2009-12-10 11:31:48 UTC
Actually I'm working on yet another different API, so I'll hold off on committing this.
Comment 6 Christian Persch 2009-12-16 18:51:47 UTC
Created attachment 149863 [details] [review]
different API
Comment 7 Christian Persch 2010-01-25 14:32:43 UTC
Created attachment 152228 [details] [review]
updated patch
Comment 8 Behdad Esfahbod 2010-01-26 05:55:37 UTC
Didn't really do a review, but any reason why some functions have two leading underscores?

I think at this point, you should commit this if you are confident in it, and make any further changes later.  Unless you'd rather postpone this to next cycle.  API freeze was, like, yesterday?
Comment 9 Christian Persch 2010-01-27 17:24:03 UTC
Most of the __vte_pty_* functions are named this way to avoid exporting them, since the _vte_pty_* functions are explicitly exported from libvte.

API freeze only applies to platform libs, which vte is not.

However this is a rather big change, so I think I'll put it on a branch first and merge it after vte branches for 2.30.
Comment 10 Behdad Esfahbod 2010-01-27 17:25:34 UTC
Sounds good.
Comment 11 Behdad Esfahbod 2010-01-27 17:25:57 UTC
Also, should we simply stop exporting _vte_pty_* for next version?
Comment 12 Christian Persch 2010-01-27 23:41:52 UTC
I'd say that's something for vte 1.0 .
Comment 13 Christian Persch 2010-01-29 22:22:45 UTC
Pushed to the "new-pty" branch.
Comment 14 Christian Persch 2010-03-17 18:31:47 UTC
Pushed to master.