Renaming non-X x_* procedures in xdisp.c (and elsewhere)

classic Classic list List threaded Threaded
57 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
It would be helpful to determine at a glance whether or not a procedure
is generic or depends on X support. I'm guessing that this naming scheme
is a historical artifact from when graphical support meant X support,
but I believe that it is more confusing than it is worth at this point.

For example, x_write_glyphs could become disp_write_glyphs.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Stefan Monnier
> It would be helpful to determine at a glance whether or not a procedure
> is generic or depends on X support. I'm guessing that this naming scheme
> is a historical artifact from when graphical support meant X support,
> but I believe that it is more confusing than it is worth at this point.

That's right.

> For example, x_write_glyphs could become disp_write_glyphs.

Along similar lines, it would be nice to make sure that the "x_" prefix
is only used for X11 support, so that there's no name clashes between
the W32, X11, and NS backends, opening up the possibility to add the
ability to support several of those backends at the same time (e.g. so
the W32 and macOS builds could also use a remote X11 display).


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
In reply to this post by Alex Gramiak
> From: Alex <[hidden email]>
> Date: Sat, 23 Mar 2019 09:07:04 -0600
>
> It would be helpful to determine at a glance whether or not a procedure
> is generic or depends on X support.

As a rule of thumb that is 99% true, everything in xdisp.c is generic,
i.e. independent of the terminal-specific implementation.  The
terminal-dependent stuff is in xterm.c/xfns.c (for X),
w32term.c/w32fns.c (for w32), nsterm.m/nsfns.m (for NS), and term.c
(for TTY).

> I'm guessing that this naming scheme is a historical artifact from
> when graphical support meant X support, but I believe that it is
> more confusing than it is worth at this point.
>
> For example, x_write_glyphs could become disp_write_glyphs.

The correct name would be something like gui_write_glyphs, since
there's tty_write_glyphs in term.c.  Although I personally fail to see
how such renaming will help anyone or anything.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Paul Eggert
Eli Zaretskii wrote:
> The correct name would be something like gui_write_glyphs, since
> there's tty_write_glyphs in term.c.  Although I personally fail to see
> how such renaming will help anyone or anything.

Renaming would make the code clearer now, and would benefit those unfamiliar
with the code (or at least to this part of the code -- and I put myself in this
category as I'm often confused by the non-X x_* names). However, renaming would
also make software archaeology more difficult.  When weighing benefit vs cost,
it partly depends on how forward-looking we want to be.

I would favor renaming, though I also see the benefits of leaving things alone.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
> Cc: [hidden email]
> From: Paul Eggert <[hidden email]>
> Date: Sat, 23 Mar 2019 09:41:42 -0700
>
> Renaming would make the code clearer now, and would benefit those unfamiliar
> with the code (or at least to this part of the code -- and I put myself in this
> category as I'm often confused by the non-X x_* names). However, renaming would
> also make software archaeology more difficult.  When weighing benefit vs cost,
> it partly depends on how forward-looking we want to be.
>
> I would favor renaming, though I also see the benefits of leaving things alone.

I don't have strong opinions about this.  Aside of making the
archeology and forensics harder, renaming will get in the way of my
personal acquaintance with the code in xdisp.c and dispnew.c, but that
alone doesn't sound like a reason to object to the change.  It will
probably also require a lot more ugly #ifdef's in the mainline code
(or calling through function pointers, not sure which is worse), and
quite a few changes in the headers to go with that.

The original long-term plan, to remind us, was not just to rename the
functions, but also to extract the common code from them so that we
have only one copy of that.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
Eli Zaretskii <[hidden email]> writes:

> As a rule of thumb that is 99% true, everything in xdisp.c is generic,
> i.e. independent of the terminal-specific implementation.  The
> terminal-dependent stuff is in xterm.c/xfns.c (for X),
> w32term.c/w32fns.c (for w32), nsterm.m/nsfns.m (for NS), and term.c
> (for TTY).

Which the naming scheme should reflect, IMO. This includes the presence
of x_* procedures in the NS/W32 code that Stefan mentioned.

> Although I personally fail to see how such renaming will help anyone
> or anything.

For example, the comment in ns_redisplay_interface that feels the
need to mention that the procedures aren't tied to X.

The part that made me make this thread was that there is no visual
distinction between the generic and X-dependent procedures in the
definition of x_redisplay_interface. It's not a huge deal, but it's not
ideal.

> The original long-term plan, to remind us, was not just to rename the
> functions, but also to extract the common code from them so that we
> have only one copy of that.

Right, but that work can be separate from the renaming of the generic
RIF procedures in xdisp.c. If it's going to happen, then IMO it should
happen sooner rather than later.

How about the attached patch? I can mention each rename individually in
the commit message if you agree.



P.S. Should x_clear_window_mouse_face instead be renamed to
clear_window_mouse_face since it doesn't depend on HAVE_WINDOW_SYSTEM
like the others do?

0001-Rename-non-X-x_-procedures-in-xdisp.c.patch (27K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
Alex <[hidden email]> writes:

> How about the attached patch? I can mention each rename individually in
> the commit message if you agree.

Sorry, I sent out an unfinished patch; here's the correct one:


0001-Rename-non-X-x_-procedures-in-xdisp.c.patch (30K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
In reply to this post by Alex Gramiak
> From: Alex <[hidden email]>
> Cc: Paul Eggert <[hidden email]>,  [hidden email]
> Date: Sat, 23 Mar 2019 11:39:49 -0600
>
> > As a rule of thumb that is 99% true, everything in xdisp.c is generic,
> > i.e. independent of the terminal-specific implementation.  The
> > terminal-dependent stuff is in xterm.c/xfns.c (for X),
> > w32term.c/w32fns.c (for w32), nsterm.m/nsfns.m (for NS), and term.c
> > (for TTY).
>
> Which the naming scheme should reflect, IMO. This includes the presence
> of x_* procedures in the NS/W32 code that Stefan mentioned.

But what you propose in the patch stops short of that goal, it just
renames the functions that are explicitly called from xdisp.c.  It
doesn't rename x_* functions in files unrelated to X.  I'm not sure
this partial renaming is worth the trouble.

> P.S. Should x_clear_window_mouse_face instead be renamed to
> clear_window_mouse_face since it doesn't depend on HAVE_WINDOW_SYSTEM
> like the others do?

It does depend on the window-system, albeit somewhat subtly: it is
only invoked for some terminal types.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
Eli Zaretskii <[hidden email]> writes:

>> From: Alex <[hidden email]>
>> Cc: Paul Eggert <[hidden email]>,  [hidden email]
>> Date: Sat, 23 Mar 2019 11:39:49 -0600
>>
>> > As a rule of thumb that is 99% true, everything in xdisp.c is generic,
>> > i.e. independent of the terminal-specific implementation.  The
>> > terminal-dependent stuff is in xterm.c/xfns.c (for X),
>> > w32term.c/w32fns.c (for w32), nsterm.m/nsfns.m (for NS), and term.c
>> > (for TTY).
>>
>> Which the naming scheme should reflect, IMO. This includes the presence
>> of x_* procedures in the NS/W32 code that Stefan mentioned.
>
> But what you propose in the patch stops short of that goal, it just
> renames the functions that are explicitly called from xdisp.c.  It
> doesn't rename x_* functions in files unrelated to X.  I'm not sure
> this partial renaming is worth the trouble.

It can be considered as just a step towards that goal. As I mentioned, I
don't see why all the work has to happen at the same time. IMO the RIF
x_* and the multiply-defined x_* are separate, even if related, issues;
the RIF x_* being much easier to solve (as demonstrated).

If you're referring to another class of x_* procedures to be renamed,
then those can be done in a later commit.

>> P.S. Should x_clear_window_mouse_face instead be renamed to
>> clear_window_mouse_face since it doesn't depend on HAVE_WINDOW_SYSTEM
>> like the others do?
>
> It does depend on the window-system, albeit somewhat subtly: it is
> only invoked for some terminal types.

If it's not invoked for non-GUI Emacs, then the gui_* prefix would
indeed be appropriate.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
> From: Alex <[hidden email]>
> Cc: [hidden email],  [hidden email]
> Date: Sat, 23 Mar 2019 12:55:26 -0600
>
> > But what you propose in the patch stops short of that goal, it just
> > renames the functions that are explicitly called from xdisp.c.  It
> > doesn't rename x_* functions in files unrelated to X.  I'm not sure
> > this partial renaming is worth the trouble.
>
> It can be considered as just a step towards that goal.  As I mentioned, I
> don't see why all the work has to happen at the same time. IMO the RIF
> x_* and the multiply-defined x_* are separate, even if related, issues;
> the RIF x_* being much easier to solve (as demonstrated).

I tend to prefer to do it in one go.  The reason is that in the past
we've seen once or twice someone who made such initial steps towards a
goal that we considered important, but then didn't follow up with the
rest, and we were left with a slightly less readable and/or slightly
less familiar code, and no real gains.  So now I'm less inclined to
support such partial steps which in themselves have little gains to
offer.

Of course, it's possible to accumulate the changes piecemeal on a
separate branch, if you want to work on this in several increments,
I'm only talking about landing them on master.

> If you're referring to another class of x_* procedures to be renamed,
> then those can be done in a later commit.

I think I see them all as a single class.  That some of them are used
through redisplay_interface is not an important distinction, IMO.

> >> P.S. Should x_clear_window_mouse_face instead be renamed to
> >> clear_window_mouse_face since it doesn't depend on HAVE_WINDOW_SYSTEM
> >> like the others do?
> >
> > It does depend on the window-system, albeit somewhat subtly: it is
> > only invoked for some terminal types.
>
> If it's not invoked for non-GUI Emacs, then the gui_* prefix would
> indeed be appropriate.

When the mouse is involved, the separation between GUI and non-GUI
tends to be blurry.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
Eli Zaretskii <[hidden email]> writes:


> I tend to prefer to do it in one go.  The reason is that in the past
> we've seen once or twice someone who made such initial steps towards a
> goal that we considered important, but then didn't follow up with the
> rest, and we were left with a slightly less readable and/or slightly
> less familiar code, and no real gains.  So now I'm less inclined to
> support such partial steps which in themselves have little gains to
> offer.

I see the tendency to prefer so, but I disagree that this would make the
code less readable or meaningfully less familiar.

>> If you're referring to another class of x_* procedures to be renamed,
>> then those can be done in a later commit.
>
> I think I see them all as a single class.  That some of them are used
> through redisplay_interface is not an important distinction, IMO.

I agree that it's not the RIF aspect that's the distinguishing factor; I
should have said "x_* procedures in the same class of x_* procedures as
those in my patch".

IMO renaming involving trivial changes (ones covered by my patch), and
renaming involving branching functionality to different windowing
systems are two distinct classes.

I'll see about making a scratch branch for the other x_* procedures.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
In reply to this post by Eli Zaretskii
Eli Zaretskii <[hidden email]> writes:

> I don't have strong opinions about this.  Aside of making the
> archeology and forensics harder, renaming will get in the way of my
> personal acquaintance with the code in xdisp.c and dispnew.c, but that
> alone doesn't sound like a reason to object to the change.  It will
> probably also require a lot more ugly #ifdef's in the mainline code
> (or calling through function pointers, not sure which is worse), and
> quite a few changes in the headers to go with that.

How about using something like the following? It's ugly, but at least
it doesn't ruin the rest of the code.

#ifdef HAVE_X_WINDOWS
#define CASE_X(proc, ...)                       \
  case output_x_window:                         \
  x_ ## proc (__VAR_ARGS__)
#define CASE_X_VAR(var, proc, ...)              \
  case output_x_window:                         \
  var = x_ ## proc (__VA_ARGS__)
#else
#define CASE_X(...)
#define CASE_X_VAR(...)
#endif

#ifdef HAVE_NTGUI
#define CASE_W32(proc, ...)                     \
  case output_w32:                              \
  w32_ ## proc (__VA_ARGS__)
#define CASE_W32_VAR(var, proc, ...)            \
  case output_w32:                              \
  var = w32_ ## proc (__VA_ARGS__)
#else
#define CASE_W32(...)
#define CASE_W32_VAR(...)
#endif

#ifdef HAVE_NS
#define CASE_NS(proc, ...)                      \
  case output_ns:                               \
  ns_ ## proc (__VA_ARGS__)
#define CASE_NS_VAR(var, proc, ...)             \
  case output_ns:                               \
  var = ns_ ## proc (__VA_ARGS__)
#else
#define CASE_NS(...)
#define CASE_NS_VAR(...)
#endif

#define CALL_FOR_WS(f, proc, ...)               \
  switch ((f)->output_method)                   \
    {                                           \
      CASE_X (proc, __VA_ARGS__);               \
      CASE_W32 (proc, __VA_ARGS__);             \
      CASE_NS (proc, __VA_ARGS__);              \
    }

#define ASSIGN_FOR_WS(f, var, proc, ...)        \
  switch ((f)->output_method)                   \
    {                                           \
      CASE_X_VAR (var, proc, __VA_ARGS__);      \
      CASE_W32_VAR (var, proc, __VA_ARGS__);    \
      CASE_NS_VAR (var, proc, __VA_ARGS__);     \
    }

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
On March 24, 2019 6:50:45 AM GMT+02:00, Alex <[hidden email]> wrote:

> Eli Zaretskii <[hidden email]> writes:
>
> > I don't have strong opinions about this.  Aside of making the
> > archeology and forensics harder, renaming will get in the way of my
> > personal acquaintance with the code in xdisp.c and dispnew.c, but
> that
> > alone doesn't sound like a reason to object to the change.  It will
> > probably also require a lot more ugly #ifdef's in the mainline code
> > (or calling through function pointers, not sure which is worse), and
> > quite a few changes in the headers to go with that.
>
> How about using something like the following? It's ugly, but at least
> it doesn't ruin the rest of the code.
>
> #ifdef HAVE_X_WINDOWS
> #define CASE_X(proc, ...)                       \
>   case output_x_window:                         \
>   x_ ## proc (__VAR_ARGS__)
> #define CASE_X_VAR(var, proc, ...)              \
>   case output_x_window:                         \
>   var = x_ ## proc (__VA_ARGS__)
> #else
> #define CASE_X(...)
> #define CASE_X_VAR(...)
> #endif
>
> #ifdef HAVE_NTGUI
> #define CASE_W32(proc, ...)                     \
>   case output_w32:                              \
>   w32_ ## proc (__VA_ARGS__)
> #define CASE_W32_VAR(var, proc, ...)            \
>   case output_w32:                              \
>   var = w32_ ## proc (__VA_ARGS__)
> #else
> #define CASE_W32(...)
> #define CASE_W32_VAR(...)
> #endif
>
> #ifdef HAVE_NS
> #define CASE_NS(proc, ...)                      \
>   case output_ns:                               \
>   ns_ ## proc (__VA_ARGS__)
> #define CASE_NS_VAR(var, proc, ...)             \
>   case output_ns:                               \
>   var = ns_ ## proc (__VA_ARGS__)
> #else
> #define CASE_NS(...)
> #define CASE_NS_VAR(...)
> #endif
>
> #define CALL_FOR_WS(f, proc, ...)               \
>   switch ((f)->output_method)                   \
>     {                                           \
>       CASE_X (proc, __VA_ARGS__);               \
>       CASE_W32 (proc, __VA_ARGS__);             \
>       CASE_NS (proc, __VA_ARGS__);              \
>     }
>
> #define ASSIGN_FOR_WS(f, var, proc, ...)        \
>   switch ((f)->output_method)                   \
>     {                                           \
>       CASE_X_VAR (var, proc, __VA_ARGS__);      \
>       CASE_W32_VAR (var, proc, __VA_ARGS__);    \
>       CASE_NS_VAR (var, proc, __VA_ARGS__);     \
>     }

Where would something like that be needed?  Can you point out a couple of places in the code where we should use this?

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
Eli Zaretskii <[hidden email]> writes:

> Where would something like that be needed? Can you point out a couple
> of places in the code where we should use this?

This would be used wherever some generic procedure needed to call a
windowing system-specific procedure. Currently this is done by all
backends defining the same x_* procedure that the generic procedure
uses. This is a method to avoid this name clash (and also getting a step
closer to the goal of using multiple backends simultaneously).

For example, in frame.c (keep_ratio), replace the x_set_offset call
with:

CALL_FOR_WS (f, set_offset, f, pos_x, pos_y, -1)

In frame.c (adjust_frame_size), replace x_set_window_size with:

CALL_FOR_WS (f, set_window_size, f, 0, new_text_width, new_text_height, 1)

In frame.c (do_switch_frame), replace x_get_focus_frame with:

ASSIGN_FOR_WS (f, xfocus, get_focus_frame, f)

I believe these two macros should cover at least most cases, but any
remaining ones should be able to be covered with a similar macro or by
manually retrieving the current frame and then using one of these
macros.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Yuri Khan
On Sun, Mar 24, 2019 at 10:07 PM Alex <[hidden email]> wrote:

> This would be used wherever some generic procedure needed to call a
> windowing system-specific procedure.

Come on, this is a typical and solved design problem. The usual
solution is called polymorphism or virtual methods, does not require a
(macro-masked) switch statement at each call site, is quite possible
in C and not only OOP languages, and is used in Emacs in at least a
few places (terminals come to mind).

1: The generic part defines function pointer types for each
WS-specific function.
2: The generic part also defines a structure type whose members are of
function pointer types defined in step 1.
3: Each WS-specific part defines implementations for each WS-specific
function, with its own name prefix, following signatures defined in
step 1.
4: Each WS-specific part defines a global structure of type defined in
step 2 and fills it out with pointers to implementations defined in
step 3.
5: The structure defined in step 4 is either global (for a
single-windowing-system build) or gets passed around (for a
multiple-window-system build).

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
> From: Yuri Khan <[hidden email]>
> Date: Sun, 24 Mar 2019 23:01:09 +0700
> Cc: Eli Zaretskii <[hidden email]>, Paul Eggert <[hidden email]>,
> Emacs developers <[hidden email]>
>
> 1: The generic part defines function pointer types for each
> WS-specific function.
> 2: The generic part also defines a structure type whose members are of
> function pointer types defined in step 1.
> 3: Each WS-specific part defines implementations for each WS-specific
> function, with its own name prefix, following signatures defined in
> step 1.
> 4: Each WS-specific part defines a global structure of type defined in
> step 2 and fills it out with pointers to implementations defined in
> step 3.
> 5: The structure defined in step 4 is either global (for a
> single-windowing-system build) or gets passed around (for a
> multiple-window-system build).

I think you have described redisplay_interface.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
In reply to this post by Alex Gramiak
> From: Alex <[hidden email]>
> Cc: Paul Eggert <[hidden email]>, [hidden email]
> Date: Sun, 24 Mar 2019 09:05:00 -0600
>
> Eli Zaretskii <[hidden email]> writes:
>
> > Where would something like that be needed? Can you point out a couple
> > of places in the code where we should use this?
>
> This would be used wherever some generic procedure needed to call a
> windowing system-specific procedure. Currently this is done by all
> backends defining the same x_* procedure that the generic procedure
> uses. This is a method to avoid this name clash (and also getting a step
> closer to the goal of using multiple backends simultaneously).
>
> For example, in frame.c (keep_ratio), replace the x_set_offset call
> with:
>
> CALL_FOR_WS (f, set_offset, f, pos_x, pos_y, -1)
>
> In frame.c (adjust_frame_size), replace x_set_window_size with:
>
> CALL_FOR_WS (f, set_window_size, f, 0, new_text_width, new_text_height, 1)

OK, thanks.

AFAICS, the vast majority of x_* functions defined in w32*.c and ns*.m
are either static or used from other w32*.c/ns*.m files, i.e. by the
same window-system back-end.  Those can be simply renamed into w32_*
and ns_*, and that's it.

The remaining small minority should probably simply be added to
redisplay_interface, and used as we do with the other functions
there.  Some of those are not literally "for redisplay", but I don't
think it matters too much.

In order for a function to be able to call through FRAME_RIF, it must
have access to the appropriate 'struct frame' pointer.  So my
suggestion is to audit all the 'extern' x_* functions which have such
multiple implementations, and see which ones have callers that don't
have access to the corresponding frame.  We then need to see how to
solve that (hopefully, in each such case we will find a way to get at
the frame somehow).

The only remaining problem that I could spot is that there's a small
number of Lisp primitives named x-SOMETHING, which are implemented by
each GUI backend.  Example: x-display-pixel-width.  I think for now we
should leave those primitives alone without renaming, and only change
their implementation to call the x_*, w32_*, or ns_* functions for
each back-end.  Renaming of these primitives can be done as a separate
step, and we will have to decide on the name pattern (something like
"xw-SOMETHING, perhaps?), and add obsolete aliases for backward
compatibility.

Are there any other issues related to this that I missed?

Thanks for working on this.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
In reply to this post by Eli Zaretskii
> Date: Sun, 24 Mar 2019 18:13:12 +0200
> From: Eli Zaretskii <[hidden email]>
> Cc: [hidden email], [hidden email], [hidden email]
>
> I think you have described redisplay_interface.

And beyond that, we shouldn't assume that everyone here who tries
their hand in working on the C sources is an experienced C
programmer.  It should be possible to describe programming techniques
without making it sound as if everyone and their grandma should be
familiar with those techniques.

Thanks.

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Alex Gramiak
In reply to this post by Eli Zaretskii
Eli Zaretskii <[hidden email]> writes:

> AFAICS, the vast majority of x_* functions defined in w32*.c and ns*.m
> are either static or used from other w32*.c/ns*.m files, i.e. by the
> same window-system back-end.  Those can be simply renamed into w32_*
> and ns_*, and that's it.

Sorry, I thought that most/all of the x_* were named that way due to
being used in generic places like image.c and frame.c since I
encountered a good few of them in those two files, but I guess it's good
that I was wrong here. I'll handle those first.

> The remaining small minority should probably simply be added to
> redisplay_interface, and used as we do with the other functions
> there.  Some of those are not literally "for redisplay", but I don't
> think it matters too much.

I would find it confusing for procedures not used for redisplay to be
used in the RIF. If there turns out to be a sizeable group of related
x_* procedures, what about adding a separate set of function pointers
for them?

> The only remaining problem that I could spot is that there's a small
> number of Lisp primitives named x-SOMETHING, which are implemented by
> each GUI backend.  Example: x-display-pixel-width.  I think for now we
> should leave those primitives alone without renaming, and only change
> their implementation to call the x_*, w32_*, or ns_* functions for
> each back-end.  Renaming of these primitives can be done as a separate
> step, and we will have to decide on the name pattern (something like
> "xw-SOMETHING, perhaps?), and add obsolete aliases for backward
> compatibility.

IMO it would be nicer to get rid of the `x' entirely, and go for `gui'
as the generic prefix, and just `ns' and `w32' as the backend prefixes.

Where would be the best place to add the obsolete aliases? For example,
I'm not sure the best place to put the obsolete call for
x-stretch-cursor (now gui-stretch-cursor).

> Are there any other issues related to this that I missed?

I noticed that several procedures in frame.c contained this comment:

/* I think this should be done with a hook.  */

Which seems to be from 1993. Do you agree with this? Several of the x_*
could be turned into these hooks.

P.S. I happen to have some code that refactors the tooltip code in
xfns.c and w32fns.c (nsfns.c's is a bit too different, unfortunately)
into generic procedures that call out to backend procedures. I figured
that it would be nicer to create a new generic file guifns.c to host
these generalizations rather than put them in a file like frame.c; would
you rather not have such a new file?

Reply | Threaded
Open this post in threaded view
|

Re: Renaming non-X x_* procedures in xdisp.c (and elsewhere)

Eli Zaretskii
> From: Alex <[hidden email]>
> Cc: [hidden email]
> Date: Sun, 24 Mar 2019 12:30:16 -0600
>
> > The remaining small minority should probably simply be added to
> > redisplay_interface, and used as we do with the other functions
> > there.  Some of those are not literally "for redisplay", but I don't
> > think it matters too much.
>
> I would find it confusing for procedures not used for redisplay to be
> used in the RIF. If there turns out to be a sizeable group of related
> x_* procedures, what about adding a separate set of function pointers
> for them?

Maybe you are right.  How about making a list of those functions
first?  When I looked at them, my impression was that most of them
_are_ related to display, but maybe I was wrong.  We can decide once
we see the list.

> > The only remaining problem that I could spot is that there's a small
> > number of Lisp primitives named x-SOMETHING, which are implemented by
> > each GUI backend.  Example: x-display-pixel-width.  I think for now we
> > should leave those primitives alone without renaming, and only change
> > their implementation to call the x_*, w32_*, or ns_* functions for
> > each back-end.  Renaming of these primitives can be done as a separate
> > step, and we will have to decide on the name pattern (something like
> > "xw-SOMETHING, perhaps?), and add obsolete aliases for backward
> > compatibility.
>
> IMO it would be nicer to get rid of the `x' entirely, and go for `gui'
> as the generic prefix, and just `ns' and `w32' as the backend prefixes.

The xw-* thing has a precedent, though.  We use gui- for selections
and such likes, but xw- for the other kind.

> Where would be the best place to add the obsolete aliases?

lisp/term/common-win.el, perhaps?

> I noticed that several procedures in frame.c contained this comment:
>
> /* I think this should be done with a hook.  */
>
> Which seems to be from 1993. Do you agree with this? Several of the x_*
> could be turned into these hooks.

Almost all of them are x_* functions which are being taken care here
anyway.  The 2 remaining ones are related to moving the mouse pointer,
and should probably be handled in the same manner.

> P.S. I happen to have some code that refactors the tooltip code in
> xfns.c and w32fns.c (nsfns.c's is a bit too different, unfortunately)
> into generic procedures that call out to backend procedures. I figured
> that it would be nicer to create a new generic file guifns.c to host
> these generalizations rather than put them in a file like frame.c; would
> you rather not have such a new file?

I think frame.c is a better place.  That file is not too large, so
adding a couple of functions to its would be okay.  Tooltips are
frames, after all.

Thanks.

123