Bug 222523 - Review Request: gmrun - A lightweight "Run program" window with TAB completion
Review Request: gmrun - A lightweight "Run program" window with TAB completion
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Patrice Dumas
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-13 02:08 EST by Gilboa Davara
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-20 06:02:44 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
notting: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Gilboa Davara 2007-01-13 02:08:21 EST
Spec URL: http://gilboadavara.thecodergeek.com/gmrun.spec
SRPM URL: http://gilboadavara.thecodergeek.com/gmrun-0.9.2-1.src.rpm

Description:
A simple program which provides a "run program" window,
featuring a bash-like TAB completion.
It uses GTK+ interface. Also, supports CTRL-R / CTRL-S / "!" for 
searching through history.

$ rpmlint -i -v gmrun-0.9.2-1.src.rpm
I: gmrun checking

- Gilboa
Comment 1 Gilboa Davara 2007-01-13 02:10:08 EST
Added NEED-REVIEW.
Comment 2 Gilboa Davara 2007-01-13 03:42:09 EST
Changed NEEDREVIEW to NEEDSPONSOR.
Comment 3 Mamoru TASAKA 2007-01-15 13:58:20 EST
Removing NEEDSPONSOR (bug 221873)
Comment 4 Gilboa Davara 2007-01-17 00:51:26 EST
%changelog
* Sat Jan 13 2007 <gilboad AT gmail DOT com> - 0.9.2-2
- Fix Source0 URL.
- Remove redundant %%doc files.

Spec URL: http://gilboadavara.thecodergeek.com/gmrun.spec
SRPM URL: http://gilboadavara.thecodergeek.com/gmrun-0.9.2-2.src.rpm

- Gilboa
Comment 5 Patrice Dumas 2007-01-17 16:36:45 EST
* A quick look at the spec file shows that Group: and URL: are remnants
  from cgdb.

* it doesn't seems to use directly glib2, so maybe the BR for gtk2-devel
  is sufficient

* in my opinion, you could remove the leading 'A' from summary, and
  'for minimal WMs'.

* Once again %{_datadir}/gmrun/gmrunrc is not to be marked with %config.

* the %{_datadir}/gmrun/ directory is unowned.

* the default config file should be tuned a bit for fedora and apropriate
  dependencies should be added. In fact I think that it could be easier
  to rewrite the config file from scratch and add the upstream one in %doc

  In my opinion, the best integration would be achieved by calling 
   xdg-open
  on every URL and on any file, whatever its extension is. Currently it
  is not possible to specify a default handler for url or files. Maybe
  upstream could be convinced to add that possibility to the config?
  In the mean time, I think that every entry in the config file should be 
  handled with xdg-open. For URL_*, could be like:

URL_http = xdg-open %u
URL_mailto = xdg-open %u
URL_man = xdg-open %u
URL_info = xdg-open %u
URL_pd = xdg-open %u
URL_file = xdg-open %u
URL_readme = xdg-open %u

  I removed URL_sh, I don't think it is that clever to handle such URI.
  Maybe some %u should in fact be %s, please adjust. 
  
  And for EXT:*:

EXT:txt,cc,cpp,h,java,html,htm,epl,tex,latex,js,css,xml,xsl,am,doc,rtf,ps,pdf =
xdg-open %s

  The terminal should better be xterm in my opinion given the target of 
  gmrun.

* There is a missing BuildRequires on popt. (Hopefully on popt-devel
  once it is split).
Comment 6 Gilboa Davara 2007-01-18 10:12:05 EST
"* A quick look at the spec file shows that Group: and URL: are remnants
  from cgdb."

Sigh.
/Bangs head against the wall.

"* it doesn't seems to use directly glib2, so maybe the BR for gtk2-devel
  is sufficient"

Indeed. gtk2-devel REQ glib2-devel. Done.

* xdg-open

Seems too heavy for me.
Correct if I'm wrong, but doesn't xdg-open requires an active browser to handle
the different URLs?
As I'm targeting, well, my Laptop (P2/366, 256MB) I'm doing my best (in
IceWM/gmrun/idesk) to remove unnecessary dep-chains and keep things
as-lean-as-possible.

* BuildRequires on popt.

Missed it in the configure output. Done.


* Thu Jan 18 2007 <gilboad AT gmail DOT com> - 0.9.2-3
- Fix wrong group and project URLs.
- Fix summery, description.
- Remove BR: glib2-devel. (REQ by gtk2-devel).
- Add BR: popt
- Remove gmrunrc from %%config section.
- Set gmrunrc to 0644 (by default it's 0600)
- Configure gmrunrc to better handle Fedora URI's.
- Add REQ: htmlview. (gmrunrc)
- Add REQ: xterm. (gmrunrc)

Spec URL: http://gilboadavara.thecodergeek.com/gmrun.spec
SRPM URL: http://gilboadavara.thecodergeek.com/gmrun-0.9.2-3.src.rpm
Comment 7 Patrice Dumas 2007-01-18 20:33:36 EST
(In reply to comment #6)

> * xdg-open
> 
> Seems too heavy for me.

xdg-open is very light, it is only a shell script. Its package (xdg-utils)
only brings in indirectly libglib-2.0 which is already a dep.

> Correct if I'm wrong, but doesn't xdg-open requires an active browser to handle
> the different URLs?

xdg-open doesn't do anything itself. It calls the right command in kde, 
gnome and xfce. Otherwise it tries mimeopen (a perl script from 
perl-File-MimeInfo) and last it tries a browser (firefox being the default
browser).

So I think using that script is in my opinion the most generic way
of handling mimetypes (it is similar with htmlview, but more generic).

Now the real question, in my opinion, is: do you want to have a 
Requires on perl-File-MimeInfo (or %{_bindir}/mimeopen), given that it
brings in perl, in order to be sure that every file will be handled
right, or do you accept poor handling of URI and files in case the 
user don't have a way to use the freedesktop stuff?

> As I'm targeting, well, my Laptop (P2/366, 256MB) I'm doing my best (in
> IceWM/gmrun/idesk) to remove unnecessary dep-chains and keep things
> as-lean-as-possible.

I also think this is a good idea.
Comment 8 Patrice Dumas 2007-01-18 20:54:44 EST
NEWS should certainly be in %doc

It is a personal preference, not something mandatory at all, but I
personally like to a a training / in %files for directories to
show visually that it is a directory and not a file. Here it leads to

%{_datadir}/gmrun/

The %attr line is wrong, root,root is missing.
Comment 9 Gilboa Davara 2007-01-20 02:13:28 EST
* xdg-open:

I stand corrected, but I tried it a couple of months ago (on my laptop) it
always seemed to start firefox when it needed to resolve a certain MIME type.
Getting a P2/366 laptop start firefox everytime you type man:pthread_create is a
-very- frustrating experience...
Now, if I could use say, elinks instead, it would have been nice.

NEWS, %{_datadir}:

Fixed.

* A couple of simple question: (More of a user-opinion)
- I'm thinking about adding xscreensaver to the dep chain.
Autostart it using startup and add Ctrl+Alt+L to activate it. (xscreensaver-command)
Do you see any problem with auto-starting xscreensaver by default?
- Second, I'm thinking about doing adding menu support to a couple of key
system-config-xxx applets. (desktop, screensaver, mouse, sound, etc)
- Third. How about adding xrdb -load ~/.Xdefaults by default?

* Sat Jan 20 2007 <gilboad AT gmail DOT com> - 0.9.2-4
- Add missing NEWS to %%doc.

Spec URL: http://gilboadavara.thecodergeek.com/gmrun.spec
SRPM URL: http://gilboadavara.thecodergeek.com/gmrun-0.9.2-4.src.rpm

- Gilboa
Comment 10 Gilboa Davara 2007-01-20 02:14:31 EST
... Sigh. I suck at multi-threading :(

Ignore the "couple of questions", they belong to icewm.

- Gilboa
Comment 11 Patrice Dumas 2007-01-21 09:43:48 EST
(In reply to comment #9)
> * xdg-open:
> 
> I stand corrected, but I tried it a couple of months ago (on my laptop) it
> always seemed to start firefox when it needed to resolve a certain MIME type.
> Getting a P2/366 laptop start firefox everytime you type man:pthread_create is a
> -very- frustrating experience...
> Now, if I could use say, elinks instead, it would have been nice.

As I said above, with perl-File-MimeInfo installed additionally things are
better. Although 
mimeopen -n 'man:pthread_create'
leads to 
Could not determine mimetype for file: man:pthread_create. Same with
urls. Ok, so I revert what I said above. xdg-open together with 
mimeopen may be good for files, not for urls.

It works for file:, and file extensions. So maybe the patch to 
gmrunrc could have

URL_file = xdg-open %s

EXT:doc,rtf,txt,cc,cpp,h,java,html,htm,epl,tex,latex,js,css,xml,xsl,am,ps,pdf =
xdg-open %s

And some requires missing are

Requires: man, info
Comment 12 Gilboa Davara 2007-01-28 09:02:43 EST
Done.

* Sun Jan 28 2007 <gilboad AT gmail DOT com> - 0.9.2-5
- Missing REQ: man.
- Missing REQ: info.
- Missing REQ: xdg-open.
- Use xdg-open to handle files.

Spec URL: http://gilboadavara.thecodergeek.com/gmrun.spec
SRPM URL: http://gilboadavara.thecodergeek.com/gmrun-0.9.2-5.src.rpm

- Gilboa
Comment 13 Patrice Dumas 2007-01-29 08:52:25 EST
The %files line:
%attr(0644,-,-) %{_datadir}/gmrun/gmrunrc

is unuseful (and harmful)...  
Comment 14 Patrice Dumas 2007-01-29 08:54:31 EST
I still think that 'for minimal WMs' shouldn't be in the summary. 
In my opinion it is wrong. gmrun may be used with any wm.
Comment 15 Gilboa Davara 2007-02-04 04:23:43 EST
Fixed.

* Sun Feb 04 2007 <gilboad AT gmail DOT com> - 0.9.2-6
- Fixed summery.
- Remove redundant fileattr.

Spec URL: http://gilboadavara.thecodergeek.com/gmrun.spec
SRPM URL: http://gilboadavara.thecodergeek.com/gmrun-0.9.2-6.src.rpm

- Gilboa
Comment 16 Patrice Dumas 2007-02-05 12:05:06 EST
* rpmlint is silent
* spec legible, follow guidelines
* free software, GPL license included
* sane provides
* source match upstream
6cef37a968006d9496fc56a7099c603c  gmrun-0.9.2.tar.gz
* %files section right

APPROVED

The source timestamps are not kept. It is not a blocker, but better.
wget -N or spectool allows to keep timestamps.

ls -l gmrun-0.9.2.tar.gz ../SOURCES/gmrun-0.9.2.tar.gz 
-rw-rw-r-- 1 dumas dumas 66097 nov 16  2003 gmrun-0.9.2.tar.gz
-rw-r--r-- 1 dumas dumas 66097 jan 15 11:28 ../SOURCES/gmrun-0.9.2.tar.gz


popt will hopefully be popt-devel someday soon.
Comment 17 Gilboa Davara 2007-02-10 05:45:11 EST
* Sat Feb 10 2007 <gilboad AT gmail DOT com> - 0.9.2-7
- Preserve the source time-stamp.

Spec URL: http://gilboadavara.thecodergeek.com/gmrun.spec
SRPM URL: http://gilboadavara.thecodergeek.com/gmrun-0.9.2-7.src.rpm
Comment 18 Patrice Dumas 2007-02-10 06:27:45 EST
This is already approved... You could have fixed the timestamp
before importing to cvs. I can confirm that it is indeed fixed.
Comment 19 Gilboa Davara 2007-02-20 06:02:44 EST
Done.

Thanks for the review.

- Gilboa
Comment 20 Christian Iseli 2007-06-05 12:27:47 EDT
Make Summary parser happy.

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