Bug 222372 - Review Request: tilda - a quake like drop down terminal for GNOME
Review Request: tilda - a quake like drop down terminal for GNOME
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-11 17:02 EST by Josef Bacik
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-04-13 14:19:28 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Josef Bacik 2007-01-11 17:02:41 EST
Spec URL: http://www.toxicpanda.com/tilda.spec
SRPM URL: http://www.toxicpanda.com/tilda-0.9.4-1.src.rpm
Description: 
Tilda is a Linux terminal taking after the likeness of many classic terminals
from first person shooter games, Quake, Doom and Half-Life (to name a few),
where the terminal has no border and is hidden from the desktop until a key is
pressed.
Comment 1 Josef Bacik 2007-01-11 17:03:30 EST
btw I'm in the middle of being sponsored by somebody, but its not happened yet,
just FYI.
Comment 2 Josef Bacik 2007-01-11 17:07:43 EST
added a BR for libconfuse-devel which is required for building

Spec URL: http://www.toxicpanda.com/tilda.spec
SRPM URL: http://www.toxicpanda.com/tilda-0.9.4-2.src.rpm
Comment 3 Michael Schwendt 2007-01-11 17:23:52 EST
> cp {AUTHORS,COPYING,README,ChangeLog,TODO,NEWS} 
> $RPM_BUILD_ROOT/%{_datadir}/doc/%{name}-%{version}

Instead of that, simply do this

  %doc AUTHORS COPYING README ChangeLog TODO NEWS

in the %files section.
Comment 4 Josef Bacik 2007-01-12 17:31:29 EST
ahh im braindead.  fixed it

Spec URL: http://www.toxicpanda.com/tilda.spec
SRPM URL: http://www.toxicpanda.com/tilda-0.9.4-3.src.rpm
Comment 5 Christoph Wickert 2007-01-19 22:04:51 EST
Looks good on a first glance. Stay tuned for a detailed review.
Comment 6 Christoph Wickert 2007-01-20 08:33:55 EST
Review for
dd8afaafb2657d6cabc43b93a7d408c8  tilda-0.9.4-3.src.rpm

FIX - rpmlint not silent on the SRPM:
W: tilda macro-in-%changelog doc
fix this by adding a second '%' to %doc: %%doc
OK - package and spec named according to the package naming guidelines
OK - package meets packaging guidelines
OK - license is open-source compatible (GPLv2)
OK - license field in spec matches actual license
OK - COPYING included in source and correctly installed in %doc
OK - spec is in American English
OK - spec is legible
OK - source in srpm matches upstream by md5
773d47e3985f7e778b662a38b053c1df
OK - package compiles and build into binaries on Core 6 i386
OK - no known ExcludeArchs
FIX - BuildRequires not correct: gettext is not required. 
FIX - ./configure checks for libXt (see config.log ~182), so you should
BuildRequire libXT-devel
Note - two redundant BRs that could be removed: glib2-devel is required by
pango-devel, gtk2-devel is pulled in by vte-devel
OK - no locales to worry about
OK - no shared libs to worry about
OK - package is not relocatable
OK - package owns all directories it creates (it doesn't create any)
OK - no duplicates in %files
NOTE - I guess tilda.png should also be installed to
%{_datadir}/icons/hicolor/48x48/apps/ for it is size specific
OK - file permissions and %defattr correct
OK - valid clean section
OK - macro usage consistent
OK - code, not content
OK - no large docs
OK - docs don't affect runtime
NOTE - You can remove the NEWS from %doc as long as it only tells you to look at
the README
OK - no header files, static libs or *.pc files
OK - no libtool archives
OK - desktop file included and correctly installed with desktop-file-install
FIX - add '--remove-category="Application"' to desktop-file-install as
"Application" no longer is a valid category according to 
http://standards.freedesktop.org/menu-spec/latest/apa.html
NOTE - you could also add '--copy-name-to-generic-name' so we get a generic name
NOTE - consider adding a comment to the desktop file. As a quick fix use
something like 'echo "Comment=A quake like terminal for GNOME" >> tilda.desktop'
during %prep
OK - package doesn't own directories already owned by other files
OK - package builds in mock (devel)
OK - basically package works as described but there is no drop down or roll up.
Also I see a critical gtk error (" Gtk-CRITICAL **: gtk_window_resize: assertion
`height > 0' failed ").

NEEDSWORK

Please fix at least the "FIX"-issues, before I can approve the package.
Comment 7 Christoph Wickert 2007-01-20 08:36:18 EST
(In reply to comment #6)
> BuildRequire libXT-devel

Should be libXt-devel, sorry for the typo.
Comment 8 Josef Bacik 2007-01-22 12:48:35 EST
Spec URL: http://www.toxicpanda.com/tilda.spec
SRPM URL: http://www.toxicpanda.com/tilda-0.9.4-4.src.rpm

fixed all the FIX issues and most of the notes.  Do you have a good suggestion
on how to copy the icon into %{_datadir}/icons/hicolor/48x48/apps/, should I
just do a %{_cp} in the %build section?  Also, what do you mean by

basically package works as described but there is no drop down or roll up.
Also I see a critical gtk error (" Gtk-CRITICAL **: gtk_window_resize: assertion
`height > 0' failed ").

I'm not getting the error, and it drop's down/rolls up for me.  You have to make
sure to setup the keybindings for it to drop down/roll up.
Comment 9 Christoph Wickert 2007-01-22 14:57:21 EST
(In reply to comment #8)
> 
> Do you have a good suggestion
> on how to copy the icon into %{_datadir}/icons/hicolor/48x48/apps/, should I
> just do a %{_cp} in the %build section? 

You could use something like:

    install -p -m 644 %{name}.png \
        $RPM_BUILD_ROOT%{_datadir}/icons/hicolor/48x48/apps/%{name}.png

> Also, what do you mean by
> 
> basically package works as described but there is no drop down or roll up.
> Also I see a critical gtk error (" Gtk-CRITICAL **: gtk_window_resize: assertion
> `height > 0' failed ").
> 
> I'm not getting the error, 

no errors when you start tilda from the commandline? Not even when you open the
prefs window?

> and it drop's down/rolls up for me.  You have to make
> sure to setup the keybindings for it to drop down/roll up.

I know, but I wasn't able to set up working keys in Gnome. I tried CTRL+F1,
STRG+F1, ALT+12 and some more, doesn't matter. And yes, I did restart tilda
every time after the changes. Nothing happens when I'm on the desktop or in
another window, when tilda is focussed, ALT+12 is interpreted as the keystrokes
";3~".

What keys are you using?

Also after setting the binding to F12 I stumbled over

$ LANG=C tilda
Key Incorrect -- Read the README or tilda.sf.net for info, rerun as 'tilda -C'
to set keybinding
: Success
Speicherzugriffsfehler

The message to run tilda -C is ok, but of course the program should not segfault.
Comment 10 Josef Bacik 2007-01-22 15:31:08 EST
Spec URL: http://www.toxicpanda.com/tilda.spec
SRPM URL: http://www.toxicpanda.com/tilda-0.9.4-5.src.rpm

for the icon change.

I'm running tilda on fluxbox, but when I use gnome and run tilda from the
command line I do see the gtk errors you are talking about.  I will look into
fixing those.  I am using Alt+5 for my keybindings, but I also tried Control+F1
and that worked as well.  The keybindings are a little finicky, set it to
"Alt+5" and see if it works.  If you don't have the exact format that it likes,
i.e. you set it to "Ctrl+F1" instead of "Control+F1" it wont work.  I will also
look into the segfault.
Comment 11 Josef Bacik 2007-01-22 16:26:06 EST
Spec URL: http://www.toxicpanda.com/tilda.spec
SRPM URL: http://www.toxicpanda.com/tilda-0.9.4-6.src.rpm

fixed the segfault.  The gtk warnings seem to be coming from VTE, nothing tilda
is doing itself so I cannot do much about those.
Comment 12 Josef Bacik 2007-01-22 16:33:29 EST
hmm, when i was building and testing the segfault went away, but when i install
the package it still segfaults... /me goes to see wtf went wrong.
Comment 13 Josef Bacik 2007-01-22 16:39:28 EST
nevermind I'm an idiot, i had tilda installed in /usr/local/bin from when i was
building/testing and such, carry on :).
Comment 14 Christoph Wickert 2007-01-22 19:13:29 EST
(In reply to comment #10)
> The keybindings are a little finicky, set it to
> "Alt+5" and see if it works. 

Argh, works. Must be lowercase, Alt instead of ALT
Comment 15 Josef Bacik 2007-01-31 13:45:13 EST
Is there anything else I need to fix?
Comment 16 Christoph Wickert 2007-01-31 13:50:38 EST
The gtk-errors can IMO be ignored, but the segfault should be fixed, at least
before this enters the stable release of fedora.
Comment 17 Josef Bacik 2007-01-31 13:54:39 EST
the segfault is fixed, I included the patch in the -6 release

Patch0:         tilda-segfault-fix.patch
Comment 18 Christoph Wickert 2007-01-31 13:58:41 EST
Ah, I see. Because of your comment #13 I thought it's still segfaulting. I will
look at this package tonight.
Comment 19 Christoph Wickert 2007-01-31 18:44:06 EST
Another quick REVIEW for

33ffdba098c3778711c17cd072e1919c  tilda-0.9.4-6.src.rpm

OK - rpmlint is silent now
OK - BuildRequires are correct now
OK - desktop file is valid and correctly installed now
OK - Package works as described: Roll up/drop down works, no more segfault. The
gtk-error can be ignored ("gtk_window_resize: assertion `height > 0' failed" is
a quite common error).

All issues are fixed so this package is ACCEPTED.

I think you should send your patch to the author of the program so this fix can
be included upstream.
Comment 20 Josef Bacik 2007-02-05 18:28:17 EST
FC-5 FC-6 tilda josef josef@toxicpanda.com
Comment 21 Josef Bacik 2007-04-13 14:19:28 EDT
Hmm I never closed this.  Its been committed to CVS and released.

Note You need to log in before you can comment on or make changes to this bug.