Re: Adding battery support on Cygwin

[Eli Zaretskii]

> From: Ken Brown <address@hidden>
> Date: Fri, 12 Jan 2018 21:33:33 -0500
> 
> The Cygwin-w32 currently supports battery status via the function 
> w32fns.c:Fw32_battery_status.  The X11 and nox builds don't have this 
> support, and Cygwin lacks the facilities used on unix-like systems to 
> provide it (/proc/apm, etc.).  But it turns out to be easy to compile 
> and use Fw32_battery_status on all Cygwin builds, simply by pulling it 
> out of w32fns.c into a new file.
> 
> The attached patch does this.  OK to push?

Yes, but see some comments below.

> If so, to which branch?

Master, of course.  And please add a NEWS entry.

> * src/w32fns.c (Fw32_battery_status): Move to a new file,
> src/w32battery.c.

Let's use a better name, because this feature is not the last one to
be used both in the w32 and Cygwin/X builds.  How about w32common.c?
Or, if this is too similar to the (unrelated) w32common.h, how about
w32cygwinx.c (since cygw32.c is already taken)?

> * src/w32battery.h: New file, containing prototype of
> syms_of_w32battery.

We don't need a new header file just to have a single prototype in
it.  You can put this prototype in lisp.h (we already have quite a few
of syms_of_* there).

> +if test "${HAVE_W32}" = "no" && test "${opsys}" = "cygwin"; then
> +  W32_LIBS="$W32_LIBS -lkernel32"
> +  W32_OBJ="$W32_OBJ w32battery.o"
> +fi

This looks like W32_OBJ and W32_LIBS have some values in the Cygwin
non-w32 build, but AFAIU those symbols are actually empty in that
case, and the above is the only place where they get any content.  So
I'd say just W32_OBJ="w32battery.o" etc.

> -#ifdef HAVE_NTGUI
> +#if defined HAVE_NTGUI
>        syms_of_w32term ();
>        syms_of_w32fns ();
>        syms_of_w32menu ();
>        syms_of_fontset ();
>  #endif /* HAVE_NTGUI */

Why was this change necessary?

Thanks.