Re: Fill column indicator functionality

[Eli Zaretskii <eliz@gnu.org>]

> Date: Mon, 18 Mar 2019 12:42:49 +0100
> From: Ergus <spacibba@aol.com>
> Cc: emacs-devel@gnu.org
> 
> 1) How to  access a specific buffer variable defined in buffer.c from
> xdisp.c Obviously there is a connection throw the 'it' struct, but maybe
> it is not the proper way to read it.

There's no need to do anything with 'struct it' for this purpose.
Just do something like

   int fc = BVAR (current_buffer, fill_column);

This works for any variable that is actually a field of 'struct
buffer'.  And when the display engine works on displaying some window,
that window's buffer is always stored in current_buffer.

> (BTW: why there is not a header file for xdisp?)

There is: dispextern.h.  The name is not xdisp.h for historical
reasons.

> That could reduce the size of xdisp.c moving all the documentation
> and comments to the header.

This is C, not C++; we don't put code commentary on header files, we
put it near the implementation, i.e. mostly in *.c files (except for
macros and inline functions that are defined in the headers).

> 2) The question about caching that was not only because of performance
> concerns, but also for code cleanup; because setting the variables in
> the redisplay call will localize the 'if' asserts to one place and
> reduce some code repeats (the potential reduce of operations /
> display_line is just a good side effect in this case.)

I don't think I understand the concern here, unless it's about what
you say below:

> The point is that right now we have code in 4 parts in this file and any
> correction requires to touch these 4 parts (add_space_at_end ,
> extend_face gui part, extend_face tui part and Lisp variable
> declarations). If anyone touches this code apart from me, the workflow
> could be very error prone, but also there are like 10-15 lines that are
> repeated in the 3 places without a real need because the values only
> change from time to time. 
> 
>   if (!NILP (Vdisplay_fill_column_indicator)
>       && (it->w->pseudo_window_p == 0)
>       && (!NILP (Vdisplay_fill_column_indicator_column))
>       && FIXNATP (Vdisplay_fill_column_indicator_character))
>     {
>        int fill_column_indicator_column = -1;
> 
>        if (EQ (Vdisplay_fill_column_indicator_column, Qt))
>          fill_column_indicator_column = fill_column;
>        else if (FIXNATP (Vdisplay_fill_column_indicator_column))
> 	     fill_column_indicator_column =
> 	       XFIXNAT (Vdisplay_fill_column_indicator_column);

You can make this a macro or a short function, and thus avoid the
repetition.  We have quite a few macros in xdisp.c, for similar
reasons.