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 75 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.