Re: [PATCH] Renaming non-X x_* identifiers

[Alex Gramiak]

Eli Zaretskii <address@hidden> writes:

>> From: Alex Gramiak <address@hidden>
>> Cc: address@hidden,  address@hidden
>> Date: Sat, 13 Apr 2019 21:35:01 -0600
>> 
>> > If every window-system is required to provide these hooks, then I
>> > think it will be enough to test only those which also have
>> > implementations on TTY frames.
>> 
>> Okay. How about wrapping the required hooks in termhooks.h in #ifdef
>> HAVE_WINDOW_SYSTEM so that terminal-only builds that erroneously have
>> these hooks in their scope issue a compiler error?
>
> I generally dislike system-dependent definitions and declarations.  I
> prefer them to be available on all systems, with some default values
> instead.  And terminal-only builds are extremely rare (I think only
> Hydra does them regularly, because all the bugs in that area are
> flagged by Hydra), so this defense is quite weak, IME.

The hooks I'm referring to are ones that AFAIU don't make any sense on
non-HAVE_WINDOW_SYSTEM, and their calls are (should be) already #ifdef'd
out (with non-window-system frames never reaching the call). Including
the #ifdefs in the declaration side would allow for Hydra to detect
cases where an undefined hook is called, which would mean adding the
appropriate #ifdef or an appropriate check around that call.

> So I think a test for the relevant hooks to be non-NULL would be a
> better alternative.  We can document in the struct definition which
> members can be NULL and in what situations.

I agree about the documentation, though I was thinking about documenting
which hooks _must_ be defined for certain types of platforms. I suppose
we could do both.

I'm a bit confused by your comment on testing. Didn't you say that it
was okay that the code that was under HAVE_WINDOW_SYSTEM didn't test for
existence of required HAVE_WINDOW_SYSTEM hooks? Those hooks are the ones
I was thinking about wrapping into #ifdefs.

>> >   1) leave those symbols alone
>> >   2) declare them obsolete, but meanwhile put both the new and the old
>> >      symbols into frame-parameters
>> >
>> > The above assumes that if a Lisp program does something with one of
>> > these parameters, that will have no effect, i.e. that these parameters
>> > are one-way communications from the Emacs internals to Lisp, as far as
>> > Lisp programs are concerned.  If the communications are two-way, then
>> > I don't see how we can change these names; do you have any ideas?
>> 
>> AFAIU it's technically possible that someone could use `put' to set a
>> new value, but that's tantamount to changing the internal definition of
>> the frame parameter setter to another frame parameter setter, so I don't
>> think such a use case should really be considered.
>> 
>> I don't have any other ideas, but 2) doesn't sound terrible as long as
>> it would be removed some day. Though I don't feel strongly about the
>> symbols here.
>
> On second thought, I think we should simply leave these alone.  They
> are just symbol names, and mostly used internally, so the problem is
> purely aesthetic and usually hidden from the view.  Doing something
> like 2) above would be an overkill for such minor issue.

Okay, I pushed a couple commits to revert these changes.