Re: [bug-gtypist] ideas, implementations, patches.

[Felix Natter]

clutton <address@hidden> writes:

> On Fri, 2016-07-08 at 17:52 +0200, Felix Natter wrote:
>> clutton <address@hidden> writes:
> [snip]
>
>> > 
>> Ok, here's a review:
>> 
>> - configure.ac: looks good, but do we need to add it to CFLAGS as
>> well?
>>   (why is CPPFLAGS honored by plain gcc anyway?)
>
> Nonono, that's not what I meant to commit, it depicts my final
> intention.I need to determine where ncursesw.h resides and add this
> directory to -I. Then I make clear branch/patch. May be you can give me
> some suggestions how to do it using autotools? (since pkg-config would
> be redundancy...) It'll save some time...

I think this comes close to it:
http://stackoverflow.com/questions/23405853/how-to-check-for-the-existence-of-a-header-of-a-package

>> 
>> - of course cmdline.[ch] and getopt.c are not under our control, but
>> I
>>   realize the changes are necessary to be able to work with -Wall.
>> 
>> - gtypist.c, infoview.c, script.h, utf8.c:
>>   If a variable isn't used, why not just remove (instead of
>> uncomment)
>>   the declaration?
>
> I think I missed that, sorry.

No problem.

>>   
>> Also, do we want to enable "-Wall" in configure.ac (maybe -ansi
>> -pedantic as well)? This might break compilation with non-gcc
>> compilers
>> though, so it's probably better to do this for development only.
>
> Indeed, it's a bad practice. Developers must build using -Wetc. Release
> shouldn't have such flags.

+1

>> 
>> Once this is fixed (should be easy), do you want to merge the branch
>> yourself (no conflicts)?
>> $ git checkout master
>> $ git merge autoall
>> $ <add+commit src/ChangeLog entry>
>
> I'll make a clean one. That branch were no more then testing and
> investigation.

+1

>> 
>> Next time it would be good to have cleaner branches (i.e. do not name
>> commits "curses.h testing").
>> 
>> [2] Tim added i18n support for GNU gengetopt, which unfortunately is
>> not
>> released yet, so I commited the files to git temporarily.
>> 
>> BTW: Shall I commit this change? I think you've said that it
>> fixes your problem?
>> 
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> -gtypist_LDADD = @LIBINTL@ # TODO: do we need @LIBICONV@ here?
>> +gtypist_LDADD = @LIBINTL@ @LIBICONV@
>
> No, wait a little, till I finish with that stupid ncursesw thing, it
> should be simpler now, I have more time.
>
>> 
>> > 
>> > Back to topic, after hacking a little I've found that nowadays
>> > projects
>> > resolve such issues by using pkg-config. With pkg-config it'll
>> > awfully
>> > simple to do proper checks. Something like:
>> > 
>> > pkg-config --cflags-only-I ncursesw
>> > pkg-config --cflags ncursesw
>> > 
>> > People hack it like that
>> > 
>> > PKG_CHECK_MODULES([NCURSES], [ncursesw], [LIBS="$LIBS
>> > $NCURSES_LIBS";
>> > ncurses=ncursesw],
>> >   [AC_CHECK_LIB([ncursesw],
>> >                [initscr],
>> >                [LIBS="$LIBS -lncursesw"; ncurses=ncursesw],
>> >                [ncurses=ncurses])
>> > ])
>> > 
>> > But, adding another dependence... What do you think? Should we left
>> > everything as it is now? Or use some hack instead of pkg-config?
>> > 
>> > ncursesw has it's own utility for such a thing:ncursesw6-
>> > config, ncursesw5-config. But I vote against using them.
>> We've discussed using pkg-config before (after David from gentoo
>> reported that cbreak() sometimes is in -ltinfo):
>> 
>> See the thread "[bug-gtypist] gtypist fails to build with
>> ncurses[tinfo]":
>> https://lists.gnu.org/archive/html/bug-gtypist/2013-09/threads.html
>> https://lists.gnu.org/archive/html/bug-gtypist/2014-01/threads.html
>> (especially the resolution:
>> https://lists.gnu.org/archive/html/bug-gtypist/2014-01/msg00009.html)
>> 
>> BTW: We also need to make sure that gentoo compilation still works,
>> but I think it should.
>
>
> Ok, I see, using pkg-config is not an option.
>
> My steps would be:
>
> 1) Proper ncursesw header inclusion, using autotools
> 2) I'll make the project build with -Wall/etc flags.
> 3) Iconv, as you suggested, although when we reach this point you could
> add it by yourself.
>
> Ok, 3 tasks. You will review and merge. Deal?

Sure, I've always done that :-)

Cheers and Best Regards,
-- 
Felix Natter