emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Question about display engine


From: Ergus
Subject: Re: Question about display engine
Date: Sat, 21 Sep 2019 23:55:51 +0200
User-agent: NeoMutt/20180716

On Sat, Sep 21, 2019 at 11:20:44AM +0300, Eli Zaretskii wrote:

Sorry for the delay.

Having looked at the code, I'm not sure I understand the problem.  Why
not simply pass the attr_filter down to the respective merge_face_ref
calls, where you currently force zero instead?  Am I missing
something?

Some other comments about your code:

. Please rename handle_face_prop_general into something like
  face_at_pos, and make it just return the face ID, without assigning
  any field in 'struct it'.  handle_face_prop will then call
  face_at_pos and assign the face ID as needed.
. handle_face_prop_general is supposed to be called just once with
  the last argument non-zero, so I see no reason why it should be
  also passed the initial_face_id argument.  It looks wrong to call
  that function with it->extend_face_id as the 2nd argument, and have
  it compute it->extend_face_id, because the value you pass as an
  argument is undefined: it hasn't been computed yet.  I think the
  function should use it->face_id internally instead of that
  argument.
. I don't understand why you need new members of 'struct it', like
  extend_face_id, saved_extend_face_id, etc.
  extend_face_to_end_of_line correctly assigns the value of extend
  face ID to it->face_id, after saving it->face_id in a local
  variable, so I see no need for it->extend_face_id, certainly not
  for it->saved_extend_face_id.  You also have extend_face_id in
  other related structures, where it is never used.

Regarding documentation: if you have difficulties with the Texinfo
markup, you could write plain text, and someone else could then add
markup.  Adding markup is a mostly mechanical procedure, unlike coming
up with a useful text.

Hi:

I just added some documentation in the reference manuals please check
it.

I should add a comment in the NEWS file, in which part is the right one?


Thanks.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]