search

OmarCastro / rofi-blocks

A Rofi modi that allows controlling rofi content throug communication with an external program
GNU General Public License v3.0
107 stars 9 forks source link

Remove unneeded use of g_strdup_printf + use of g_spawn_ + popup error. #1

Closed DaveDavenport closed 4 years ago

DaveDavenport commented 4 years ago

I don't think a full g_strdup_printf if needed in these situations.

DaveDavenport commented 4 years ago

Sorry wanted this to be split, but forgot to push to separate branch.

2nd commit removes the popen2/execl in favour of glib g_spawn_async function. It also solves a minor memory leak with pd->cmd.

It also popup an small message when failed to launch.

OmarCastro commented 4 years ago

No problem, these contributions are great nonetheless! With popen2/execl the reason of failure is shown on the rofi modal's message block to help in cases rofi is launched outside command prompt (e.g. keyboard shortcut on a Desktop Environment), however, the amount of launch error situations are expected to be so small it is acceptable to be shown on commad prompt, I am going to change to print on stderr instead of stdout .

OmarCastro commented 4 years ago

Seems like g_spawn_async_with_pipes does not trigger on_child_status when child exits, I am going to revert this part to popen2/execl.

DaveDavenport commented 4 years ago

my bad.. missed that one :(

DaveDavenport commented 4 years ago

probably needs the: G_SPAWN_DO_NOT_REAP_CHILD flag


G_SPAWN_DO_NOT_REAP_CHILD | the child will not be automatically reaped; you must use g_child_watch_add()yourself (or call waitpid() or handle SIGCHLD yourself), or the child will become a zombie.
-- | --
OmarCastro commented 4 years ago

Bumping this only to save a note for future reference:

Now that I had the time to test it, G_SPAWN_DO_NOT_REAP_CHILD fixed it, changed back to g_spawn. Just had to make some changes to maintain the same behavior when failing to load script.