[wdiff-bugs] Re: wdiff (was: Re: gnulib-cache.m4 readability)

[Bruno Haible]

Hi Denver,

> >  1) there should be a release on ftp.gnu.org that does not dump core at
> >     every invocation, (*)
> 
> Yes, there should and there will be one soon (see below).

Ah, great! 

> wdiff 0.5.93 is now available (at http://alpha.gnu.org/gnu/wdiff/),
> which includes a plethora of fixes since 0.5.90.  The plan is to
> release 0.6 within the next month.

I take this as a "request for testing". Feedback below.

> >  2) the program should support colorized output like we know it from webcvs
> >     and others.
> 
> By "the program", do you mean wdiff or some web-based diff viewer?

I meant the wdiff program. It has an option -t which can produce underlined/
reversed video in terminal emulators. Why not also colors? That's done with
escape sequences, like the underlining/reverse video.

<propaganda>
I would recommend to use the same mechanism as the 'msgcat' program, which
allows the user to specify the colors in the output freely, in well-known
CSS syntax.
  http://www.gnu.org/software/gettext/manual/html_node/msgcat-Invocation.html
  
http://www.gnu.org/software/gettext/manual/html_node/The-_002d_002dcolor-option.html
  
http://www.gnu.org/software/gettext/manual/html_node/The-_002d_002dstyle-option.html
I'm planning to move this facility out of GNU gettext into an independent
library.
</propaganda>

> But if you can make a convincing case for colorized output in wdiff,
> perhaps I could add it.

The main argument is that it will save other programs from parsing the
wdiff output. (Can it be parsed unambiguously at all?)


Now for some little things that would be worth improving before the 0.6
release:

* The config.guess, config.sub infrastructure is one year old.

* The gnulib infrastructure in lib/ and m4/ appears to be also at least
  10 months old.

* The configure file was generated with autoconf-2.61, which leads to weird
  configure output on MacOS X 10.5:

   checking for alloca as a compiler built-in... rm: cannot remove 
`conftest.dSYM': Is a directory
   yes

  This can be fixed by using autoconf-2.62.

* What's up with the ChangeLogs? You say, there are many fixes, but
  src/ChangeLog ends in 1999.

* There are many warnings when compiling with "gcc -Wall":

  $ ./configure CPPFLAGS=-Wall
  $ make

gcc -DLOCALEDIR=\"/usr/local/share/locale\" -DHAVE_CONFIG_H -I. -I..  -I../lib 
-Wall  -g -O2 -MT unify.o -MD -MP -MF .deps/unify.Tpo -c -o unify.o unify.c
unify.c: In function `main':
unify.c:486: warning: suggest parentheses around && within ||
unify.c:489: warning: suggest parentheses around && within ||
unify.c:489: warning: suggest parentheses around && within ||
unify.c:334: warning: `type' might be used uninitialized in this function

gcc -DLOCALEDIR=\"/usr/local/share/locale\" -DHAVE_CONFIG_H -I. -I..  -I../lib 
-Wall  -g -O2 -MT wdiff.o -MD -MP -MF .deps/wdiff.Tpo -c -o wdiff.o wdiff.c
wdiff.c: In function `reformat_diff_output':
wdiff.c:861: warning: suggest explicit braces to avoid ambiguous `else'
wdiff.c:867: warning: suggest explicit braces to avoid ambiguous `else'
wdiff.c:900: warning: suggest explicit braces to avoid ambiguous `else'
wdiff.c: In function `complete_input_program':
wdiff.c:973: warning: implicit declaration of function `wait'

gcc -DLOCALEDIR=\"/usr/local/share/locale\" -DHAVE_CONFIG_H -I. -I..  -I../lib 
-Wall  -g -O2 -MT mdiff.o -MD -MP -MF .deps/mdiff.Tpo -c -o mdiff.o mdiff.c
mdiff.c: In function `study_input':
mdiff.c:1124: warning: suggest parentheses around + or - inside shift
mdiff.c:1145: warning: suggest parentheses around + or - inside shift
mdiff.c:1151: warning: suggest parentheses around + or - inside shift
mdiff.c:1142: warning: suggest explicit braces to avoid ambiguous `else'
mdiff.c:1160: warning: suggest parentheses around + or - inside shift
mdiff.c:1163: warning: suggest parentheses around + or - inside shift
mdiff.c:1168: warning: suggest parentheses around + or - inside shift
mdiff.c:1173: warning: suggest parentheses around + or - inside shift
mdiff.c:1069: warning: `buffer' might be used uninitialized in this function
mdiff.c:1070: warning: `length' might be used uninitialized in this function
mdiff.c: In function `reference_string':
mdiff.c:1332: warning: type defaults to `int' in declaration of `next'
mdiff.c: In function `explain_member':
mdiff.c:1616: warning: unused variable `counter'
mdiff.c: In function `prepare_clusters':
mdiff.c:1657: warning: unused variable `sorter'
mdiff.c: In function `prepare_indirects':
mdiff.c:1902: warning: unused variable `input'
mdiff.c:1903: warning: unused variable `duplicated_items'
mdiff.c: In function `explain_group':
mdiff.c:2014: warning: unused variable `input'
mdiff.c: In function `prepare_mergings':
mdiff.c:2218: warning: suggest explicit braces to avoid ambiguous `else'
mdiff.c:2241: warning: too many arguments for format
mdiff.c:2070: warning: `best_cost' might be used uninitialized in this function
mdiff.c: In function `output_characters':
mdiff.c:2553: warning: unused variable `counter'
mdiff.c: In function `complete_output_program':
mdiff.c:2841: warning: implicit declaration of function `wait'
mdiff.c: In function `make_margin':
mdiff.c:2896: warning: enumeration value `EMPTY_MARGIN' not handled in switch
mdiff.c: In function `relist_annotated_files':
mdiff.c:3227: warning: suggest explicit braces to avoid ambiguous `else'
mdiff.c:3135: warning: unused variable `file'
mdiff.c:3132: warning: `input' might be used uninitialized in this function
mdiff.c:3138: warning: `member' might be used uninitialized in this function
mdiff.c: In function `relist_merged_lines':
mdiff.c:3475: warning: unused variable `chosen_cluster'
mdiff.c:3476: warning: unused variable `reference'
mdiff.c: In function `relist_merged_words':
mdiff.c:3586: warning: unused variable `chosen_cluster'
mdiff.c:3587: warning: unused variable `reference'
mdiff.c: In function `main':
mdiff.c:4079: warning: unused variable `input'
mdiff.c: At top level:
mdiff.c:316: warning: `width' defined but not used
mdiff.c:349: warning: `report_identical_files' defined but not used
mdiff.c:1532: warning: `dump_all_members' defined but not used
mdiff.c:1613: warning: `explain_member' defined but not used

gcc -DLOCALEDIR=\"/usr/local/share/locale\" -DHAVE_CONFIG_H -I. -I..  -I../lib 
-Wall  -g -O2 -MT wdiff2.o -MD -MP -MF .deps/wdiff2.Tpo -c -o wdiff2.o wdiff2.c
wdiff2.c: In function `main':
wdiff2.c:351: warning: implicit declaration of function `execvp'

* The -t option is not functioning even on normal Linux systems:

  $ ./wdiff -t
  ./wdiff: Cannot use -t, termcap not available.

  Reason: wdiff.c tests HAVE_TPUTS but config.h.in does not have an entry for
  it. That is because configure.ac does not test for it. And if it did, it
  would need to link against -lncurses or (old-style) -ltermcap.

  You can find an autoconf macro and substitute code for -lncurses/-ltermcap
  in the GNU gettext repository, as gnulib modules 'termcap-h', 'termcap' in
  http://cvs.savannah.gnu.org/viewvc/gettext/gnulib-local/modules/?root=gettext
  The autoconf macro is termcap.m4 in
  http://cvs.savannah.gnu.org/viewvc/gettext/gnulib-local/m4/?root=gettext
  (You can use these files by checking out the gettext CVS and then using
  gnulib-tool --local-dir option pointing to gettext's gnulib-local directory.)

* The #ifdefs at the head of wdiff.c and in system.h can also well use some
  modernization.

Bruno