GNU bug report logs -
#77544
(WIP) [PATCH] Prettify page separators.
Previous Next
Full log
Message #17 received at 77544 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> --- a/src/xdisp.c
>> +++ b/src/xdisp.c
>> @@ -8430,6 +8430,34 @@ get_next_display_element (struct it *it)
>> int lface_id = 0;
>> int escape_glyph;
>>
>> + if (c == '\f' && it->current_x == it->first_visible_x)
>
> IMO, this condition is incorrect. You should test for the previous
> character being a newline. The it->first_visible_x condition will be
> wrong in a continuation line, when the window is hscrolled, and when
> display-line-numbers-mode is turned on, as well as in some other
> situations (line/wrap-prefix etc.).
>
>> + {
>> + const int save_face_id = it->face_id;
>> + const int save_char = it->char_to_display;
>> + const int save_current_x = it->current_x;
>> + const int save_ascent = it->ascent;
>> + const int save_descent = it->descent;
>> +
>> + face_id =
>> + merge_faces (it->w, Qtrailing_whitespace, 0, it->face_id);
>> + XSETINT (it->ctl_chars[0], 0);
>> + ctl_len = 0;
>> +
>> + it->char_to_display = '-';
>> + it->face_id = face_id;
>> + for ( ; it->current_x - it->end_charpos <= it->last_visible_x; it->current_x++)
>
> This condition is also incorrect, IMO: it->current_x is the pixel
> coordinate, whereas it->end_charpos is a character position in the
> buffer, so you cannot meaningfully subtract them. Also, I think this
> will be incorrect when the window has display margins.
>
> The right way to figure out the usable width is to call the function
> window_box_width or window_box_right_offset.
>
> These wrong conditions are probably the reasons for at least part of
> the problems you see with this patch.
I'll take it into account. Thanks.
>> + {
>> + PRODUCE_GLYPHS (it);
>> + }
>
> I think it's not a good idea to call PRODUCE_GLYPHS inside
> get_next_display_element.
I originally wanted to put this in display_line, however,
get_next_display_element was overriding the character face so i thought
that adding it to get_next_display_element (where ^L is displayed)
would be a better solution.
> Alternatively, we could implement this entirely in the
> terminal-specific back-end of the display engine (xterm.c, w32term.c,
> etc.), where we could detect the "^L" glyph sequence and replace it
> with a horizontal line of a suitable width and thickness. That would
> allow us to produce a separator that is much better-looking than just
> showing a long line of dashes.
The dashes are temporal, I'm planning to add a variable that store
the character to display (like display-fill-column-indicator-character).
I don't think it would be better display a line instead a character, in
that case, I would have preferred to use a face with a strike-through.
This should work for both Terminal and GUI frames.
> In any case, this must be an optional feature, so we will need a user
> option to control it, by default off.
Yeah, this will be buffer-local and have an user option (not minor mode)
for enable/disable it (disabled by default), i didn't added the options
yet because first i was testing if this works well, the same for the
face used.
[Message part 2 (text/html, inline)]
[Message part 3 (text/plain, inline)]
--
- E.G via GNU Emacs and Org.
This bug report was last modified today.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.