[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Fill column indicator functionality
From: |
Eli Zaretskii |
Subject: |
Re: Fill column indicator functionality |
Date: |
Mon, 18 Mar 2019 19:10:47 +0200 |
> Date: Mon, 18 Mar 2019 12:42:49 +0100
> From: Ergus <address@hidden>
> Cc: address@hidden
>
> 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.
- Re: Fill column indicator functionality, (continued)
Re: Fill column indicator functionality, Drew Adams, 2019/03/15
Re: Fill column indicator functionality, Ergus, 2019/03/15
Re: Fill column indicator functionality, Ergus, 2019/03/17