lanoxx / tilda

A Gtk based drop down terminal for Linux and Unix
GNU General Public License v2.0
1.28k stars 161 forks source link

no-return-in-nonvoid-function in tilda.c and tilda_window.c #78

Closed micrococo closed 10 years ago

micrococo commented 10 years ago

Hi.

Those errors prevents me from building the package for openSUSE in the Build Service. I think that the fix must be easy, but I don't have a clue of C/GTK and don't know how to correct this. Could you please fix this so we can have an updated Tilda version for the next openSUSE release? Package freeze will come in one or two weeks and the current version in the repositories is too old (0.9.6).

Thanks in advance.

Greetings.

lanoxx commented 10 years ago

I need some more information! Which version are you building, git master or the latest stable version? In which function/line does the error occur?

micrococo commented 10 years ago

Hi.

Yups, I forgot the details... sorry.

Tilda version is 1.1.12 (latest stable AFAIK). Line numbers are tilda.c:508 (function static int migrate_config_files) and tilda_window.c:510 (function static gboolean delete_event_callback).

Greetings.

lanoxx commented 10 years ago

I have made a commit and pushed it to tilda-1-1-test branch. Please check again with these changes, on my system it compiles fine but so does the latest stable. If the Build system of OpenSuse modifies the compile flags then there might be other compile issues besides this one. You should check if there is an option to use the compile flags as they are configured by the automake build.

lanoxx commented 10 years ago

If the build works then I can make a new release of tilda if necessary.

micrococo commented 10 years ago

Hi.

Thank you very much for your quick response! I've applied a patch with your changes and now it builds like a charm. Now I'm stuck with two patches that my distro applies to supposedly fix something wrong with your code. I don't understand a thing, so I've asked for help in the mailing lists. If you're curious you can see the patches at https://build.opensuse.org/package/show/GNOME:Apps/tilda. tilda-fix-pointers.patch is supposed to fix a 64bit portability issue in key_grabber.c, and tilda-fix-gdk-x11-window-set-user-time.patch comments a line of code in key_grabber.c too. They're for the 0.9.6 version and that's why I'm asking for help in the mailing list, because I don't know to update the patch for the current code.

Update: I've been told by the author of the patches that they probably may be deleted because they're too old and you may have fixed this in the new versions.

If the build works then I can make a new release of tilda if necessary.

I don't want to interfere with your release program. If you think that it is the time for a new one go for it, I'll be glad with a newer version because I'm sure you'll have fixed some issue and included something interesting.

P.S.: Now that the package builds I can see another warning: W: incorrect-fsf-address /usr/share/doc/packages/tilda/COPYING. Just to let you know.

Greetings.

lanoxx commented 10 years ago

Hi,

yes that seems to be case:

The tilda-fix-pointers.patch should be obsolete. I have been running on 64bit for a long time and did not have any issues so far. So is the fix-glib-include.patch. The X11 time stuff is something that I also don't completely understand. I have added some fixes regarding timings to 1.1.x, you might want to test whether or not the fix-gdk-x11-window-set-user-time is still necessary.

I think I also remember a fix regarding the icon path, please check if the icon patch is still required.

micrococo commented 10 years ago

Hi.

Definitely, tilda-fix-pointers.patch isn't needed anymore. It was reported at http://sourceforge.net/p/tilda/bugs/26/ but the error is not present in the current builds. I think that tilda-fix-gdk-x11-window-set-user-time.patch isn't necessary because I'm using Tilda without that patch and it's working fine. Regarding the .desktop file, it was reported at http://sourceforge.net/p/tilda/bugs/25/, and I think that it is better not to use the full path and, for the icon, no extension too. Let the system use the PATH for the executable and the search algorithm for the icon. The check for XRandR is something I don't understand. It makes no difference if I apply the patch or not... I think I'll remove it too.

I seems that there're two new issues: Show Notebook Border isn't working and Tilda steals the focus (if you launch an application an it appears on top of Tilda's window, moving the pointer over Tilda's window makes it move to the top and hide the other window). Tested with Tilda 1.1.12, LXDE and the window managers Openbox and IceWM. Can you reproduce this behaviour? For sure, Always on Top is not checked.

Greetings.

lanoxx commented 10 years ago

There is already an issue about the notebook border, there is already an issue for the notebook border at https://github.com/tsloughter/tilda/issues/7, with a solution. If that does not work for you then report a separate issue.

About the focus stealing, that issue is already closed in tilda master, I need to look into it, maybe I can backport that fix.

lanoxx commented 10 years ago

Please apply the following fix from tilda master: https://github.com/lanoxx/tilda/commit/e86fe7b299c82ec0cd1d4fe9ff72ee135934d1d0 That should fix the focus stealing.

If it works I will backport it and make a new release.

micrococo commented 10 years ago

Hi.

I'm using Tilda 1.1.2 with the patch for the openSUSE Build Service (https://github.com/lanoxx/tilda/tree/tilda-1-1-test) and I cannot apply that patch. mouse_enter function is this:

static void mouse_enter (GtkWidget *widget, GdkEvent *event, gpointer data)
{
    DEBUG_FUNCTION ("mouse_enter");
    DEBUG_ASSERT (data != NULL);
    DEBUG_ASSERT (widget != NULL);

    tilda_window *tw = TILDA_WINDOW(data);
    stop_auto_hide_tick(tw);
    if (tw->disable_auto_hide == FALSE)
        tilda_window_set_active(tw);
}

The line GdkEventCrossing *ev = (GdkEventCrossing*)event; is in the mouse_leave function. Am I doing something wrong?

Greetings.

lanoxx commented 10 years ago

Ah right, the patch does not apply automatically, but you can just manually remove the following two lines:

if (tw->disable_auto_hide == FALSE)
    tilda_window_set_active(tw);

that should do it.

lanoxx commented 10 years ago

Btw. you should be using tilda 1.1.12 not 1.1.2, or was that a typo?

micrococo commented 10 years ago

Hi.

Yes, it was a typo. Ok, the patch seems to work, thank you. There's one thing I'm not sure. If I launch an application from Konsole, when the application finishes Konsole recovers the focus. If I do the same from Tilda I have to click on its window for it to recover the focus. Hide when Tilda loses focus and Hide Tilda when mouse leaves it are not set. Is this the expected behaviour? Not a problem, just curiosity.

The notebook border looks like a theme issue as commented in the issue you pointed to, not a bug. It isn't showed with Adwaita (GTK2/GTK3) but it works with Clearlooks, Crux, Gilouche, Industrial, Mist, Raleigh, Redmond, Synchronicity and ThinIce (they're all GTK2 AFAIK).

Greetings.

lanoxx commented 10 years ago

I have updated the Tilda wiki page with some of the CSS that are mentioned in the issue that I linked earlier. Feel free to add more examples: https://github.com/lanoxx/tilda/wiki

About the focus issue, I cannot reproduce that here, when I start an application from Tilda and then close that application, then Tilda will have focus again.

micrococo commented 10 years ago

Hi.

About the focus issue, I cannot reproduce that here, when I start an application from Tilda and then close that application, then Tilda will have focus again.

Ok, you might have fixed this in a later release or it can be a local issue. I'll check it better and open a new issue if necessary. Thanks.

Greetings.

lanoxx commented 10 years ago

1.1.13 is out, closing this issue.