Re: Fill column indicator functionality

[Ergus <spacibba@aol.com>]

[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]

Hi Eli:

Here I add a WIP patch for the current implementation so far.

Please tell me any problem you see related or not with my questions to
fix them too. 

Best,
Ergus



On Tue, Mar 12, 2019 at 06:19:34PM +0200, Eli Zaretskii wrote:
>> Date: Tue, 12 Mar 2019 16:29:30 +0100
>> From: Ergus <spacibba@aol.com>
>> Cc: emacs-devel@gnu.org
>>
>> Sorry for being so annoying.
>
>Nothing to be sorry about.
>
>> I tried:
>>     if (!row->reversed_p)
>> 	{
>> 	  while (glyph >= start
>> 	         && (glyph->type == CHAR_GLYPH
>> 	             || glyph->type == STRETCH_GLYPH)
>> 		 && NILP (glyph->object))
>> 	    --glyph;
>> 	}
>>
>> But it didn't work. The behavior is the same: the whitespace is
>> highlighted only when the line is crossed.
>
>I guess at this point I'd need to see the code to help you more.
>
>Alternatively, you could step with a debugger through the code of
>highlight_trailing_whitespace and see why it isn't working.  It should
>be something simple, I think.
>
>> The other corner case I have is because in graphical mode the space for
>> the dot is always reserved, so when the last character in the line is
>> just before the line, the line is not drawn for that line.
>
>Sorry, I don't understand: what is "the dot" in this context, and what
>do you mean by "just before the line"?  There are too many "lines" in
>this sentence, so I'm confused.
>
>> In text mode I fixed this changing a while for a do while, but in
>> graphical mode the approach is different.
>
>Well, it sounds like again seeing the code should explain what I don't
>understand.
>

[-- Attachment #2: display-fill-column-indicator_wip.patch --]
[-- Type: text/plain, Size: 11139 bytes --]

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index baa05d0a89..441d3d5a9b 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -648,6 +648,15 @@ since it could result in memory overflow and make Emacs crash."
                                   (const :tag "Count lines from beginning of narrowed region"
                                          :value nil))
                                  "26.1")
+
+             (display-fill-column-indicator-column display-fill-column-indicator
+                                 (choice
+                                  (const :tag "Dynamically computed"
+                                         :value fill-column)
+                                  (integer :menu-tag "Fixed number of columns"
+                                           :value 70
+                                           :format "%v"))
+                                 "27.1")
 	     ;; xfaces.c
 	     (scalable-fonts-allowed display boolean "22.1")
 	     ;; xfns.c
diff --git a/lisp/display-fill-column-indicator.el b/lisp/display-fill-column-indicator.el
new file mode 100644
index 0000000000..d8f73193af
--- /dev/null
+++ b/lisp/display-fill-column-indicator.el
@@ -0,0 +1,71 @@
+;;; display-fill-column-indicator.el --- interface for display-fill-column-indicator -*- lexical-binding: t -*-
+
+;; Copyright (C) 2017-2019 Free Software Foundation, Inc.
+
+;; Maintainer: emacs-devel@gnu.org
+;; Keywords: convenience
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Provides a minor mode interface for `display-fill-column-indicator'.
+;;
+;; Toggle display of line numbers with M-x
+;; display-fill-column-indicator-mode.  To enable line numbering in
+;; all buffers, use M-x global-display-fill-column-indicator-mode.  To
+;; change the default line column
+
+
+;; NOTE: Customization variables for `display-fill-column-indicator'
+;; itself are defined in cus-start.el.
+
+;;; Code:
+
+(defgroup display-fill-column-indicator nil
+  "Display line numbers in the buffer."
+  :group 'convenience
+  :group 'display)
+
+
+;;;###autoload
+(define-minor-mode display-fill-column-indicator-mode
+  "Toggle display fill column indicator.
+This uses `display-fill-column-indicator' internally.
+
+To change the position of the line displayed by default,
+customize `display-fill-column-indicator-column'."
+  :lighter nil
+  (if display-fill-column-indicator-mode
+      (progn
+        (setq display-fill-column-indicator t)
+        (unless display-fill-column-indicator-column
+          (setq display-fill-column-indicator-column fill-column)))
+    (setq display-fill-column-indicator nil)))
+
+(defun display-fill-column-indicator--turn-on ()
+  "Turn on `display-fill-column-indicator-mode'."
+  (unless (or (minibufferp)
+              (and (daemonp) (null (frame-parameter nil 'client))))
+    (display-fill-column-indicator-mode)))
+
+;;;###autoload
+(define-globalized-minor-mode global-display-fill-column-indicator-mode
+  display-fill-column-indicator-mode display-fill-column-indicator--turn-on)
+
+(provide 'display-fill-column-indicator)
+
+;;; display-fill-column-indicator.el ends here
diff --git a/lisp/frame.el b/lisp/frame.el
index dd1d5b030f..c0a0c3a903 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2663,6 +2663,8 @@ See also `toggle-frame-maximized'."
         display-line-numbers-width
         display-line-numbers-current-absolute
         display-line-numbers-widen
+        display-fill-column-indicator
+        display-fill-column-indicator-column
         bidi-paragraph-direction
         bidi-display-reordering))
 
diff --git a/lisp/ldefs-boot.el b/lisp/ldefs-boot.el
index 0e8e5f699b..1227ceb377 100644
--- a/lisp/ldefs-boot.el
+++ b/lisp/ldefs-boot.el
@@ -7734,7 +7734,44 @@ See `display-line-numbers-mode' for more information on Display-Line-Numbers mod
 
 \(fn &optional ARG)" t nil)
 
-(if (fboundp 'register-definition-prefixes) (register-definition-prefixes "display-line-numbers" '("display-line-numbers-")))
+(if (fboundp 'register-definition-prefixes)
+    (register-definition-prefixes "display-line-numbers" '("display-line-numbers-")))
+
+;;;***
+\f
+;;;### (autoloads nil "display-fill-column-indicator" "display-fill-column-indicator.el"
+;;;;;;  (0 0 0 0))
+;;; Generated autoloads from display-fill-column-indicator.el
+
+(autoload 'display-fill-column-indicator-mode "display-fill-column-indicator" "\
+Toggle display fill column indicator.
+This uses `display-fill-column-indicator' internally.
+
+To change the position of the line displayed by default,
+customize `display-fill-column-indicator-column'.
+
+\(fn &optional ARG)" t nil)
+
+(defvar global-display-fill-column-indicator-mode nil "\
+Non-nil if Global Display-fill-column-indicator mode is enabled.
+See the `global-display-fill-column-indicator-mode' command
+for a description of this minor mode.")
+
+(custom-autoload 'global-display-fill-column-indicator-mode
+                 "display-fill-column-indicator" nil)
+
+(autoload 'global-display-fill-column-indicator-mode
+  "display-fill-column-indicator" "\
+Toggle display fill column indicator.
+This uses `display-fill-column-indicator' internally.
+
+To change the position of the line displayed by default,
+customize `display-fill-column-indicator-column'.
+
+\(fn &optional ARG)" t nil)
+
+(if (fboundp 'register-definition-prefixes)
+    (register-definition-prefixes "display-fill-column-indicator" '("display-fill-column-indicator-")))
 
 ;;;***
 \f
diff --git a/src/xdisp.c b/src/xdisp.c
index 6d30afda6d..928949f031 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -20305,7 +20305,8 @@ extend_face_to_end_of_line (struct it *it)
 #ifdef HAVE_WINDOW_SYSTEM
       && !face->stipple
 #endif
-      && !it->glyph_row->reversed_p)
+      && !it->glyph_row->reversed_p
+      && NILP (Vdisplay_fill_column_indicator))
     return;
 
   /* Set the glyph row flag indicating that the face of the last glyph
@@ -20359,6 +20360,47 @@ extend_face_to_end_of_line (struct it *it)
 	    }
 	}
 #ifdef HAVE_WINDOW_SYSTEM
+
+      if (!NILP (Vdisplay_fill_column_indicator))
+        {
+	  const int fill_column_indicator_column =
+		  XFIXNAT (Vdisplay_fill_column_indicator_column) - 1;
+
+	  const int current_x = it->current_x;
+	  const int column_x = it->pixel_width * fill_column_indicator_column
+	                       + it->lnum_pixel_width;
+
+	  if ((current_x <= column_x)
+	      && (column_x < it->last_visible_x))
+	    {
+	      struct font *font = face->font ? face->font : FRAME_FONT (f);
+	      const char saved_char = it->char_to_display;
+	      const struct text_pos saved_pos = it->position;
+	      const bool saved_avoid_cursor = it->avoid_cursor_p;
+	      const int saved_face_id = it->face_id;
+	      const bool saved_box_start = it->start_of_box_run_p;
+	      const int width = column_x - current_x;
+
+	      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;
+	    }
+        }
       if (it->glyph_row->reversed_p)
 	{
 	  /* Prepend a stretch glyph to the row, such that the
@@ -20478,10 +20520,34 @@ extend_face_to_end_of_line (struct it *it)
 	it->face_id = default_face->id;
       else
 	it->face_id = face->id;
-      PRODUCE_GLYPHS (it);
 
-      while (it->current_x <= it->last_visible_x)
-	PRODUCE_GLYPHS (it);
+      /* Display fill-column-line if mode is active */
+      if (!NILP (Vdisplay_fill_column_indicator))
+	{
+	  const int fill_column_indicator_line =
+	    XFIXNAT (Vdisplay_fill_column_indicator_column) + it->lnum_pixel_width;
+	  do
+	    {
+	      if (it->current_x == fill_column_indicator_line)
+	        {
+		  const int saved_face = it->face_id;
+		  it->face_id = merge_faces (it->w, Qline_number, 0, DEFAULT_FACE_ID);
+		  it->c = it->char_to_display = '|';
+		  PRODUCE_GLYPHS (it);
+		  it->face_id = saved_face;
+		  it->c = it->char_to_display = ' ';
+	        }
+	      else
+		PRODUCE_GLYPHS (it);
+	    } while (it->current_x < it->last_visible_x);
+	}
+      else
+	{
+          do
+	    {
+	      PRODUCE_GLYPHS (it);
+            } while (it->current_x < it->last_visible_x);
+        }
 
       if (WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0
 	  && (it->glyph_row->used[RIGHT_MARGIN_AREA]
@@ -20571,14 +20637,16 @@ highlight_trailing_whitespace (struct it *it)
       if (!row->reversed_p)
 	{
 	  while (glyph >= start
-		 && glyph->type == CHAR_GLYPH
+	         && (glyph->type == CHAR_GLYPH
+	             || glyph->type == STRETCH_GLYPH)
 		 && NILP (glyph->object))
 	    --glyph;
 	}
       else
 	{
 	  while (glyph <= start
-		 && glyph->type == CHAR_GLYPH
+		 && (glyph->type == CHAR_GLYPH
+	             || glyph->type == STRETCH_GLYPH)
 		 && NILP (glyph->object))
 	    ++glyph;
 	}
@@ -33213,6 +33281,19 @@ either `relative' or `visual'.  */);
   DEFSYM (Qdisplay_line_numbers_widen, "display-line-numbers-widen");
   Fmake_variable_buffer_local (Qdisplay_line_numbers_widen);
 
+  DEFVAR_LISP ("display-fill-column-indicator", Vdisplay_fill_column_indicator,
+    doc: /* Non-nil means display the fill column indicator line.  */);
+  Vdisplay_fill_column_indicator = Qnil;
+  DEFSYM (Qdisplay_fill_column_indicator, "display-fill-column-indicator");
+  Fmake_variable_buffer_local (Qdisplay_fill_column_indicator);
+
+  DEFVAR_LISP ("display-fill-column-indicator-column", Vdisplay_fill_column_indicator_column,
+    doc: /* Column where to draw the column indicator line when display-column-indicator
+is non-nil .  */);
+  Vdisplay_fill_column_indicator_column = Qnil;
+  DEFSYM (Qdisplay_fill_column_indicator_column, "display-fill-column-indicator-column");
+  Fmake_variable_buffer_local (Qdisplay_fill_column_indicator_column);
+
   DEFVAR_BOOL ("inhibit-eval-during-redisplay", inhibit_eval_during_redisplay,
     doc: /* Non-nil means don't eval Lisp during redisplay.  */);
   inhibit_eval_during_redisplay = false;