unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: agrambot@gmail.com, emacs-devel@gnu.org
Subject: Re: [RFC] Using a display_info union instead of a typedef Display_Info
Date: Wed, 17 Apr 2019 19:55:35 +0300	[thread overview]
Message-ID: <834l6wfu60.fsf@gnu.org> (raw)
In-Reply-To: <813c7912-e0fd-57aa-37c4-a1e1974e1434@cs.ucla.edu> (message from Paul Eggert on Tue, 16 Apr 2019 11:54:57 -0700)

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 16 Apr 2019 11:54:57 -0700
> Cc: emacs-devel@gnu.org
> 
> Thanks, this looks like a step in the right direction. I have a few
> minor quibbles: function names like U_DISPLAY_INFO_X shouldn't be
> lowercase, and more important DISPLAY_INFO/display_info should be a
> function and not a macro, and it should return a pointer to new generic
> type, 'struct generic_display_info' say, that contains only the leading
> elements common to all the structures - and all the structures should be
> changed to have a first member of that generic type. See the first
> members of 'struct Lisp_Vector' and 'struct Lisp_Char_Table' for an
> example of this.

I actually don't see why we need a function.  FRAME_DISPLAY_INFO can
continue being a macro, because all it does is return a pointer to a
struct which is a member of 'struct frame'.  The struct to which
FRAME_DISPLAY_INFO will have to change to support several
window-systems in the same session, but the change can be very simple,
like what we do with 'struct font' which is then extended by the
various font backends, see for example w32font.h.  Then when some code
needs to access window-system specific parts of display_info, it will
cast the pointer returned by FRAME_DISPLAY_INFO to the correct struct,
and use the result.

> I have my doubts about installing this in master now, as it doesn't have
> direct benefit now. Ironically, one of the patches I have in mind (which
> should improve overall Emacs performance significantly) involves
> reverting a patch installed some years ago that was intended to be a
> refactoring to do later work that was never done. So I appreciate Eli's
> concern about merging this into master right away.
> 
> However, the patch seems to be a good candidate for installing into a
> scratch branch that later patches can improve on, and once that branch
> has direct benefits we could merge that into master.

IMO, if we don't want to install such changes on master soon, there's
no reason to ask Alex to work on these changes.  After all, all the
ideas, the discussions, and even the patches Alex proposed are all in
the archives, so if someone will want to work on this in the future,
they can pick up where we leave off.

FWIW, if the changes are cleaned up, and for example don't introduce
dispatch functions with platform-specific #ifdef's, but instead go the
way I described above, I'm okay with considering something like that
for master.  I didn't like the original implementation, but I don't
reject the idea itself.



  reply	other threads:[~2019-04-17 16:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-12 16:50 [RFC] Using a display_info union instead of a typedef Display_Info Alex Gramiak
2019-04-12 17:06 ` Alex Gramiak
2019-04-16 18:54   ` Paul Eggert
2019-04-17 16:55     ` Eli Zaretskii [this message]
2019-04-12 17:55 ` Eli Zaretskii
2019-04-12 18:40   ` Alex Gramiak
2019-04-12 19:23     ` Eli Zaretskii
2019-04-12 19:36       ` Daniel Colascione
2019-04-12 19:44         ` Eli Zaretskii
2019-04-12 19:55           ` Daniel Colascione
2019-04-12 20:12             ` Eli Zaretskii
2019-04-14  1:17             ` Richard Stallman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=834l6wfu60.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=agrambot@gmail.com \
    --cc=eggert@cs.ucla.edu \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).