Red Hat Bugzilla – Bug 212704
Review Request: manedit - UNIX Manual Page Editor
Last modified: 2007-11-30 17:11:46 EST
Spec URL: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/manedit.spec
SRPM URL: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/manedit-0.7.1-1.src.rpm
ManEdit is a UNIX manual page editor and viewer,
it is designed specifically for the editing of the
UNIX manual page format using an integrated XML interface.
Note: manedit got orphaned on 2005-04-14 according to
Created attachment 139670 [details]
simplify a bit spec file and add patches
Created attachment 139671 [details]
add MANBASE for the man pages base directory and simplify
Created attachment 139672 [details]
use MANBASE, and use X11BASE not only on FreeBSD
Some simplification in patch, renamed without freeBSD.
The LOCALBASE, in my opinion should remain along /usr/local,
it is for the locally installed man pages.
Thank you for patches.
Well, some files are installed in wrong directory in -1 srpm and
I fixed it.
Nice application. If patrice is not interested to sponsor this package then i
can do full review and approve this package.
You can do full review, I'll comment now and then.
* a desktop file for manview is missing
* in manedit/images, there are is a serie of pixmaps for the
viewer (mp_viewer*) and for manedit (manedit*) for the hicolor
theme, to be used instead of the images in pixmaps, with the
approriate snippets used too.
* manwrap don't seem to be needed, it is only used on freebsd,
you should just remove it. If you keep it, it would require in
my opinion a patch to handle also bzip2 files, and also gzcat
should be replaced with zcat.
* there is a dependency on man, used to transform to text.
groff don't seem to be needed, however.
* I have tested and there seems to be some issues with utf8
in the viewer (not in the editor). Look at
the ' is garbled in programmer's manual
Created attachment 139821 [details]
also use LOCALBASE outside of freebsd
I updated the manbase patch since I discovered while testing that
LOCALBASE was still not used, so not FHS compliant path was used.
Can you upload new package by making changes as suggested by Patrice Dumas so
that i can review this package?
Okay I will check a new patch after I take a supper.
I think there is bug in displaying character ’ in manview for any man page
is that gtk library problem>
Indeed. Also I tried to use accented letters in the editor and it
converts the utf8 characters literally into latin1 characters.
Same if I try to view/edit man pages in the fr locale.
This is something that should be fixed, since almost all the localized
man pages use utf8 nowadays in fedora.
It is certainly linked with the fact that gtk1 don't use UTF8 in
the default case, there is a need to set the locale or the like.
I think this should be discussed with upstream.
Forgot to say that the locale issue is not a blocker in my
opinion (but may be for other reviewers), but contacting
upstream should be a must in that case, and it wouldn't be
bad to add a sentence in the description, otherwise some user
may garble their localized man pages.
I would appreciate it if someone would gtk-2-ize this package,
which will be a lot of help.
I already tried to look for gtk2ize, not deeply as makefiles are not friendly
from top DIR but failed to understand anymore.
Porting to gtk2 is a task for upstream, if I'm not wrong there
are API incompatibilities. Otherwise the changes in Makefile
should be fairly minor, maybe changing gtk-config to
pkg-config gtk+-2.0 should be enough. I have just tested that it
breaks the build, with issues with GtkText.
(In reply to comment #17)
> I already tried to look for gtk2ize, not deeply as makefiles are not friendly
> from top DIR but failed to understand anymore.
(In reply to comment #18)
> Porting to gtk2 is a task for upstream, if I'm not wrong there
> are API incompatibilities. <snip>
> I have just tested that it
> breaks the build, with issues with GtkText.
In fact I tried to gtk2ize before submitting this bug as you did, Patrice,
however it failed. I noticed that it needs not a few changes to deal with
fonts, which includes some (or many?) API change, and I gave up........
Anyway I will re-check the patch proposed and resubmit.
SPEC and SRPM are updated.
Currently I don't know how to deal with apostrophe issue.
For apostrophe issue (I don't know if this is a bug of manedit or gtk+)
and gtk2ization, I mailed to perhaps-upstream.
In my opinion, the apostrophe issue is also an utf8 issue, the
apostrophe used isn't the ascii apostrophe but an utf8 encoded
apostrophe (which is believed to be better looking).
Well, perhaps the current way is
* wait for upstream's response (I think upstream is learfox[at]twu.net
according to http://wolfpack.twu.net/contacts.html .....)
* or add some comments like "this is gtk+ package and some (especially UTF-8)
characters will appear as garbages")
I think upstream must be contacted about that issue, but I
don't think we should wait for this issue to be solved before
accepting manedit in fedora extras. the comment in %description
seems enough to me.
Okay, I added about some comments in %%description stage.
As I commented about, I mailed upstream about gtk2ization.
Well, what should I do for this package?
The spec file seems right to me, however I have spotted some compilers
warnings that are worrisome, and for some of them certainly easy to
There are many warnings about signedness:
pointer targets in passing argument 2 of '__builtin___strncpy_chk' differ in
I think this could be investigated. I don't know exactly what is the
potential impact of such warnings, but I guess in some cases there
is room for trouble.
Some include files are certainly not used as they should, I think this
should be fixed, as it is easy to fix and it may uncover other issues:
editor.c:199: warning: implicit declaration of function 'memcpy'
editor.c:199: warning: incompatible implicit declaration of built-in function
There are also these warnings:
editorop.c:1146: warning: ignoring return value of 'mkstemp', declared with
maneditop.c:566: warning: ignoring return value of 'mkstemp', declared with
viewerfio.c:177: warning: ignoring return value of 'mkstemp', declared with
viewerfio.c:181: warning: ignoring return value of 'mkstemp', declared with
And lastly (but maybe this should be better sent upstream)
prefop.c: In function 'PrefDoApply':
prefop.c:107: warning: 'gdk_color.pixel' is used uninitialized in this function
There are other warnings, but they don't seem to be problematic
Okay, I will check the warnings later.
Well, would you check the following?
(In reply to comment #29)
> Well, would you check the following?
Everything is allright now, nice!
+ package builds in mock (development i386) for FC6.
+ rpmlint is silent for RPM and SRPM.
+ source files match upstream.
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written but NOT properly indented.
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible. License text included in package.
+ %doc is small; no -doc subpackage required.
+ %doc does not affect runtime.
+ COPYING included in %doc.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ no .pc files.
+ no -devel subpackage exists
+ no .la files.
+ no translations are available.
+ owns the directories it creates.
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Desktop file installed successfully
+ Desktop file is handled correctly in SPEC file.
+ GUI application
Need your suggestion whether this package can be approved with rendering
problem in manview? I think we can accept this package and let the rendering
problem be solved in next release.
No problem with accepting, the utf8 support issue is clearly
an upstream issue.
Don't Forget to CLOSE this bug once you finish building package.
* Rebuild for FE-devel succeeded.
* SyncNeeded is requested for FE-6 (FE-5 branch already exists)
* Add manedit to owners.list, comps files
* Declare that manedit is unorphaned
Thank you for reviewing and approving this package. Now I
close this bug as CLOSED NEXTRELEASE.
NOTE: I have not yet received any response from upstream.