From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Fill column indicator functionality Date: Mon, 11 Mar 2019 17:30:47 +0200 Message-ID: <83y35l4ee0.fsf@gnu.org> References: <8736p0nznz.fsf@tcd.ie> <837eebsmaj.fsf@gnu.org> <87sgwvco1l.fsf@Ergus.i-did-not-set--mail-host-address--so-tickle-me> <83r2cel3qf.fsf@gnu.org> <20190211165636.ch5x4wb2ibdt2dzy@Ergus> <83ef8el03u.fsf@gnu.org> <20190308185744.a4vnfoab5wdvqyny@Ergus> <83y35p871q.fsf@gnu.org> <20190309132207.w2ho3j6p5on6fyzw@Ergus> <838sxo87gc.fsf@gnu.org> <20190311104814.kp2nv6arv47hcykz@Ergus> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="2214"; mail-complaints-to="usenet@blaine.gmane.org" Cc: emacs-devel@gnu.org To: Ergus Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Mar 11 16:33:45 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1h3Mvw-0000TQ-V5 for ged-emacs-devel@m.gmane.org; Mon, 11 Mar 2019 16:33:45 +0100 Original-Received: from localhost ([127.0.0.1]:35267 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h3Mvv-0005yC-R2 for ged-emacs-devel@m.gmane.org; Mon, 11 Mar 2019 11:33:43 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:42203) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h3Mvk-0005vg-4J for emacs-devel@gnu.org; Mon, 11 Mar 2019 11:33:32 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:38259) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h3MtF-0002Ka-2D; Mon, 11 Mar 2019 11:30:57 -0400 Original-Received: from [176.228.60.248] (port=2104 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1h3MtE-0005ye-GN; Mon, 11 Mar 2019 11:30:56 -0400 In-reply-to: <20190311104814.kp2nv6arv47hcykz@Ergus> (message from Ergus on Mon, 11 Mar 2019 11:48:14 +0100) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:234071 Archived-At: > Date: Mon, 11 Mar 2019 11:48:14 +0100 > From: Ergus > Cc: emacs-devel@gnu.org > > 1) In the graphical implementation I generated the column following the > Eli suggestions with something like: > > struct font *font = face->font ? face->font : FRAME_FONT (f); > > it->char_to_display = '|'; > int stretch_ascent = (((it->ascent + it->descent) > * FONT_BASE (font)) / FONT_HEIGHT (font)); > > memset (&it->position, 0, sizeof it->position); > it->avoid_cursor_p = true; > it->face_id = merge_faces (it->w, Qline_number, 0, DEFAULT_FACE_ID); > it->start_of_box_run_p = false; > append_stretch_glyph (it, Qnil, width, > it->ascent + it->descent, stretch_ascent); > > PRODUCE_GLYPHS (it); > > it->position = saved_pos; > it->avoid_cursor_p = saved_avoid_cursor; > it->face_id = saved_face_id; > it->start_of_box_run_p = saved_box_start; > it->char_to_display = saved_char; > > The problem is that in this case if (setq show-trailing-whitespace t) > the whitespaces aren't highlighted if they are not longer that the line; > in term mode (with the loop) it works fine. > > The function highlight_trailing_whitespace checks for glyph->type == > STRETCH_GLYPH so I am missing something. I think you are missing this: /* If last glyph is a space or stretch, and it's trailing whitespace, set the face of all trailing whitespace glyphs in IT->glyph_row to `trailing-whitespace'. */ if ((row->reversed_p ? glyph <= start : glyph >= start) && BUFFERP (glyph->object) <<<<<<<<<<<<<<<<<<<<<<<<<<<<<< && (glyph->type == STRETCH_GLYPH || (glyph->type == CHAR_GLYPH && glyph->u.ch == ' ')) && trailing_whitespace_p (glyph->charpos)) This test is for those stretch glyphs that came from a buffer, i.e. those generated by TABs and 'space' display properties. But in your case, the object of the stretch glyph is nil, as specified by the 2nd argument of append_stretch_glyph: append_stretch_glyph (it, Qnil, width, it->ascent + it->descent, stretch_ascent); Moreover, I believe you are looking at the wrong code fragment to explain why trailing-whitespace stopped working. I think the problem happens a bit earlier, here: /* Skip over glyphs inserted to display the cursor at the end of a line, for extending the face of the last glyph to the end of the line on terminals, and for truncation and continuation glyphs. */ if (!row->reversed_p) { while (glyph >= start && glyph->type == CHAR_GLYPH && NILP (glyph->object)) --glyph; } This skips only glyphs of type CHAR_GLYPH, but your change inserts a STRETCH_GLYPH, which you also want to skip. So the above condition should be augmented to include STRETCH_GLYPH as well. > 2) What name do you suggest for this mode? I propose > display-column-indicator. display-fill-column-indicator-mode? Thanks again for working on this. P.S. Thanks to you, I just uncovered an old bug, whereby show-trailing-whitespace didn't work in R2L screen lines, and for the same reason: the code didn't account for the stretch glyphs inserted by the display engine at the left edge of such lines.