Created attachment 63018 [details] [review]
Michael Rickmann's solution

I like the part of the patch which goes around the gnome_help_display call ; the part for the menu entries doesn't look right : it should basically just move the #ifndef DISABLE_GNOME some lines down, from just above the help entry to just below it (and above the about entry).

In the menu entries part I have rearranged things into first "contents" then "about" handled independantly by the #if s. This was necessary in order to keep things short and to consider DISABLE_GNOME && !WIN32 cases as well.
Regards
Michael

I changed that patch as you suggested. DISABLE_GNOME && !WIN32 seems not very common and if somebody tries with that combination he will just have an empty help_cb signal function.
Regards
Michael

Created attachment 63476 [details] [review]
enable help for Win32

Could you explain why you try the same thing twice ? I would think that if it fails the first time, then we just abandon...

Oh yes. Help will not be available in all languages. I thought it may be cute to install always locale C. So the first time the routine tries to find help in the locale returned by Glib. If it fails, it tries to open DATADIR\help\C\index.xhtml. We do not have an en-locale directory in ekiga-cvs. So for all English speakers the routine will succeed with the second trial.
Regards
Michael

I'd rather have twice the same code with a clear :
<first try>
if (fail)
  <second try>
than that unclear for loop.

Also, if the first try fails:
1. don't you leak the first locale ?
2. don't you free the static "C" locale ?

I agree, it was too twisted. A new version follows, everything unrolled.
Regards Michael

Created attachment 63537 [details] [review]
unrolled win32 help patch

I like this one, applying with a little modification : I'll do the memory management like this:
1)
index_path = ...;
hinst = ...;
g_free (index_path);
(no need to keep index_path longer than that)
2) use g_free (locale); just after the use of locale too.

Closing the bug on a last question : we take only two chars for the locale ; are we sure it will be enough ?

Oh you have already applied the patch to CVS. There is still an error in it. The string returned by g_win32_getlocale is not g_free ed. Sorry for having been so slow. I have been away for two days.
The more I have been thinking about your final question the more I think that we should account for fully qualified locale directories. In Ekiga's locale directory, there are already contributions using full locale and the manual is not that old yet. When you think of China, there may be quite a lot of users who need a long locale to distinguish between simplified (zh_CN) and traditional Chineese (zh_HK, zh_TW). Things are more complicated than I originally thought. Glib and the environment set by the Gtk+ installer or by the user play a role.
g_win32_getlocale (in Glib's gwin32.c) first checks the environment variables LC_ALL, LC_MESSAGES and LANG. If it finds something it is returned immediately without any further checking. This is the case on my XP Home laptop where "LANG" is set to "de". It was set during installation of Gtk+ runtime. If g_win32_getlocale does not find a language environment it calls GetThreadLocale () and constructs a Posix like locale. The returned string may be of the form "en" or "en_CA" depending on what GetThreadLocale returns and which language subidentifiers have been defined by Microsoft. In a few cases g_win32_getlocale uses 3 letter first language parts when defined by MS.
I have written a new help_cb which allows for long locales. Sorry that the patch is a bit difficult to read now as it reverses the old one.
Regards
Michael

Created attachment 63690 [details] [review]
win32 help with long locale directory names

Eh... I though g_win32_getlocale was returning a const... my bad!

About the new patch: do we really have to put the second if in the first if ? Can't we just leave them one after the other ?

Something like :
if I have a locale
  try with it
if the try failed and I have a short locale
  try with the short locale
if the try failed
  try with the C locale
?

We can have it sequentially as you said. It works.

I applied your patch, put it sequentially then committed. Thanks again!