[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Question about display engine
From: |
Eli Zaretskii |
Subject: |
Re: Question about display engine |
Date: |
Sun, 29 Sep 2019 13:57:22 +0300 |
> Date: Sun, 29 Sep 2019 12:30:34 +0200
> From: Ergus <address@hidden>
> Cc: address@hidden, address@hidden
>
> > . I don't understand why you changed the underlined_p and
> > underline_type stuff in struct face. What were the reasons for
> > that part of the changeset? Its sole effect seems to be to
> > generate some redundant changes, or am I missing something?
> >
> Use double variable to describe the same is an error prone
> practice. This change simplifies the checks in all the code related (as
> there will be only one variable to set/check), reduces one name and
> member in the struct and avoids errors related to changing one value and
> not the other.
That's your stylistic preference, and I can understand it. But when
the only reason for a change is stylistic preferences, I generally
prefer to leave the code intact, so that the original authors' version
remains as long as it does TRT.
> > . In this hunk from append_space_for_newline, why did you change
> > FACE_FROM_ID_OR_NULL to FACE_FROM_ID?
> >
> > - int local_default_face_id =
> > + int char_width = 1;
> > +
> > + if (default_face_p
> > +#ifdef HAVE_WINDOW_SYSTEM
> > + || FRAME_WINDOW_P (it->f)
> > +#endif
> > + )
> > + {
> > + const int local_default_face_id =
> > lookup_basic_face (it->w, it->f, DEFAULT_FACE_ID);
> > struct face* default_face =
> > - FACE_FROM_ID_OR_NULL (it->f, local_default_face_id);
> > + FACE_FROM_ID (it->f, local_default_face_id);
> >
>
> The call is made after a lookup_basic_face and it's for DEFAULT_FACE_ID
> is it needed the check? Normally for other faces if it is NULL then we
> use the DEFAULT_FACE_ID. In this case it should be unneeded right?
It's the other way around: FACE_FROM_ID could trigger an assertion
violation, where FACE_FROM_ID_OR_NULL will silently return a NULL
pointer. So we should only use FACE_FROM_ID where we are 110% certain
it can never violate the assertion, or where a NULL pointer for a face
will cause worse problems than an assertion.
Thanks.
- Re: Question about display engine, (continued)
- Re: Question about display engine, Eli Zaretskii, 2019/09/15
- Re: Question about display engine, Ergus, 2019/09/15
- Re: Question about display engine, Ergus, 2019/09/16
- Re: Question about display engine, Eli Zaretskii, 2019/09/17
- Re: Question about display engine, Eli Zaretskii, 2019/09/21
- Re: Question about display engine, Ergus, 2019/09/21
- Re: Question about display engine, Ergus, 2019/09/21
- Re: Question about display engine, Ergus, 2019/09/26
- Re: Question about display engine, Eli Zaretskii, 2019/09/28
- Re: Question about display engine, Ergus, 2019/09/29
- Re: Question about display engine,
Eli Zaretskii <=
Re: Question about display engine, Eli Zaretskii, 2019/09/06
- Re: Question about display engine, Ergus, 2019/09/06
- Re: Question about display engine, Eli Zaretskii, 2019/09/06
- Re: Question about display engine, Ergus, 2019/09/06
- Re: Question about display engine, Eli Zaretskii, 2019/09/06
- Re: Question about display engine, Ergus, 2019/09/06
- Re: Question about display engine, Eli Zaretskii, 2019/09/07