[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Adding battery support on Cygwin
From: |
Eli Zaretskii |
Subject: |
Re: Adding battery support on Cygwin |
Date: |
Sat, 13 Jan 2018 10:11:22 +0200 |
> 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.