Re: Fill column indicator functionality

[Ergus <spacibba@aol.com>]

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

Here is a patch with Eli's fixes and corrections:

Actually I have 3 more questions:

1) In the graphical mode the character I use now (?\u2502) does not look
like a vertical line (there is a small vertical space between them so
the line now looks like with ?|); in terminal it looks fine. What's the
proper way to change the font size or the character itself to fit the
entire row height in the graphical mode?

2) Are there any extra considerations needed to use the character
?\u2502 ? Is it needed to check (for example) if the actual fonts can
display it properly?

3) Actually there are 3 possible configuration options the column
number, the character, and the font. Should we provide some other
alternative?

The next step is to implement the R2L version of this, but I don't have
used R2L editing ever, so it may take a while.

On Wed, Mar 13, 2019 at 10:09:42PM +0200, Eli Zaretskii wrote:
>> Date: Wed, 13 Mar 2019 21:02:25 +0100
>> From: Ergus <spacibba@aol.com>
>> Cc: emacs-devel@gnu.org
>>
>> >  Lisp_Object save_object = it->object;
>> >  it->object = Qnil;
>> >
>> >before the call to PRODUCE_GLYPHS, and then restore it->object to its
>> >previous value after PRODUCE_GLYPHS.
>> >
>> I thought that append_stretch_glyph was putting the it->object to Now I
>> see it didn't.
>
>append_stretch_glyph only does that for the stretch glyph it creates,
>it doesn't change it->object.
>
>> 1E6 Thanks.
>
>My pleasure.
>

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

diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index baa05d0a89..3f58eac63b 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -648,6 +648,11 @@ 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
+                                 integer "27.1")
+             (display-fill-column-indicator-character display-fill-column-indicator
+                                 character "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..846466a2c3
--- /dev/null
+++ b/lisp/display-fill-column-indicator.el
@@ -0,0 +1,74 @@
+;;; 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' you can change the
+character for the line setting `display-fill-column-indicator-character'."
+  :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))
+        (unless display-fill-column-indicator-character
+          (setq display-fill-column-indicator-character ?\u2502)))
+    (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/faces.el b/lisp/faces.el
index ab6c384c80..153e6a208f 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2502,6 +2502,20 @@ unwanted effects."
   :group 'basic-faces
   :group 'display-line-numbers)
 
+;; Definition stolen from linum.el.
+(defface fill-column
+  '((t :inherit (shadow default)))
+  "Face for displaying fill column indicator line.
+This face is used when `display-fill-column-indicator-mode' is
+non-nil.
+
+If you customize the font of this face, make sure it is a
+monospaced font, otherwise the line's characters will not line
+up horizontally."
+  :version "27.1"
+  :group 'basic-faces
+  :group 'display-fill-column-indicator)
+
 (defface escape-glyph
   '((((background dark)) :foreground "cyan")
     ;; See the comment in minibuffer-prompt for
diff --git a/lisp/frame.el b/lisp/frame.el
index dd1d5b030f..03c4d0761b 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2663,6 +2663,9 @@ 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
+        display-fill-column-indicator-character
         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..8ac4be8dc7 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -20135,15 +20135,41 @@ append_space_for_newline (struct it *it, bool default_face_p)
 	  it->what = IT_CHARACTER;
 	  memset (&it->position, 0, sizeof it->position);
 	  it->object = Qnil;
-	  it->c = it->char_to_display = ' ';
 	  it->len = 1;
 
+	  int local_default_face_id = lookup_basic_face (it->w, it->f, DEFAULT_FACE_ID);
+	  struct face* default_face =
+	    FACE_FROM_ID_OR_NULL (it->f, local_default_face_id);
+
+	  /* Corner case for when display-fill-column-indicator-mode
+	     is active and the extra character should be added in the
+	     same place than the line */
+	  if (!NILP (Vdisplay_fill_column_indicator)
+	      && FIXNATP (Vdisplay_fill_column_indicator_column)
+	      && FIXNATP (Vdisplay_fill_column_indicator_character))
+	    {
+	       struct font *font = default_face->font ? default_face->font : FRAME_FONT (it->f);
+	       const int char_width = font->average_width ? font->average_width : font->space_width;
+	       const int fill_column = XFIXNAT (Vdisplay_fill_column_indicator_column);
+	       const int column_x = char_width * fill_column + it->lnum_pixel_width;
+
+	       if (it->current_x == column_x)
+	         {
+	           it->c = it->char_to_display = XFIXNAT(Vdisplay_fill_column_indicator_character);
+	           it->face_id = merge_faces (it->w, Qfill_column, 0, DEFAULT_FACE_ID);
+	           face = FACE_FROM_ID(it->f, it->face_id);
+	           goto produce_glyphs;
+	         }
+	    }
+
+	  it->c = it->char_to_display = ' ';
 	  /* If the default face was remapped, be sure to use the
 	     remapped face for the appended newline.  */
 	  if (default_face_p)
-	    it->face_id = lookup_basic_face (it->w, it->f, DEFAULT_FACE_ID);
+	    it->face_id = local_default_face_id;
 	  else if (it->face_before_selective_p)
 	    it->face_id = it->saved_face_id;
+
 	  face = FACE_FROM_ID (it->f, it->face_id);
 	  it->face_id = FACE_FOR_CHAR (it->f, face, 0, -1, Qnil);
 	  /* In R2L rows, we will prepend a stretch glyph that will
@@ -20152,11 +20178,12 @@ append_space_for_newline (struct it *it, bool default_face_p)
 	     set.  */
 	  if (it->glyph_row->reversed_p
 	      /* But if the appended newline glyph goes all the way to
-		 the end of the row, there will be no stretch glyph,
-		 so leave the box flag set.  */
+	         the end of the row, there will be no stretch glyph,
+	         so leave the box flag set.  */
 	      && saved_x + FRAME_COLUMN_WIDTH (it->f) < it->last_visible_x)
-	    it->end_of_box_run_p = false;
+		  it->end_of_box_run_p = false;
 
+	produce_glyphs:
 	  PRODUCE_GLYPHS (it);
 
 #ifdef HAVE_WINDOW_SYSTEM
@@ -20305,7 +20332,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 +20387,55 @@ extend_face_to_end_of_line (struct it *it)
 	    }
 	}
 #ifdef HAVE_WINDOW_SYSTEM
+
+      if (!NILP (Vdisplay_fill_column_indicator)
+          && FIXNATP (Vdisplay_fill_column_indicator_column)
+          && FIXNATP (Vdisplay_fill_column_indicator_character))
+        {
+
+	  struct font *font = default_face->font ? default_face->font : FRAME_FONT (f);
+	  const int char_width = font->average_width ? font->average_width : font->space_width;
+
+	  const int fill_column = XFIXNAT (Vdisplay_fill_column_indicator_column);
+
+	  const int column_x = char_width * fill_column + it->lnum_pixel_width;
+
+	  if ((it->current_x < column_x)
+	      && (column_x < it->last_visible_x))
+	    {
+	      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;
+	      Lisp_Object save_object = it->object;
+
+	      /* The stretch width needs to considet the latter added glyph */
+	      const int stretch_width = column_x - it->current_x - char_width;
+
+	      int stretch_ascent = (((it->ascent + it->descent)
+		    * FONT_BASE (font)) / FONT_HEIGHT (font));
+
+	      it->char_to_display = XFIXNAT(Vdisplay_fill_column_indicator_character);
+	      memset (&it->position, 0, sizeof it->position);
+	      it->avoid_cursor_p = true;
+	      it->face_id = merge_faces (it->w, Qfill_column, 0, DEFAULT_FACE_ID);
+	      it->start_of_box_run_p = false;
+	      it->object = Qnil;
+
+	      append_stretch_glyph (it, Qnil, stretch_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;
+	      it->object = save_object;
+	    }
+        }
       if (it->glyph_row->reversed_p)
 	{
 	  /* Prepend a stretch glyph to the row, such that the
@@ -20478,10 +20555,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, Qfill_column, 0, DEFAULT_FACE_ID);
+		  it->c = it->char_to_display = XFIXNAT(Vdisplay_fill_column_indicator_character);
+		  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 +20672,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;
 	}
@@ -32645,6 +32748,9 @@ be let-bound around code that needs to disable messages temporarily. */);
   /* Name of a text property which disables line-number display.  */
   DEFSYM (Qdisplay_line_numbers_disable, "display-line-numbers-disable");
 
+  /* Names of the faces used to display fill column indicator character. */
+  DEFSYM (Qfill_column, "fill-column");
+
   /* Name and number of the face used to highlight escape glyphs.  */
   DEFSYM (Qescape_glyph, "escape-glyph");
 
@@ -33213,6 +33319,27 @@ 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.  */);
+  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 to draw the indicator when `display-fill-column-indicator' is non-nil.
+The default value is the variable `fill-column' if not other value is given. */);
+  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_LISP ("display-fill-column-indicator-character", Vdisplay_fill_column_indicator_character,
+    doc: /* Character to draw the indicator when `display-fill-column-indicator' is non-nil.
+The default is ?\u2502 the but a good alternative is (ascii 124) if
+yours font doesn't support unicode characters.  */);
+  Vdisplay_fill_column_indicator_character = Qnil;
+  DEFSYM (Qdisplay_fill_column_indicator_character, "display-fill-column-indicator-character");
+  Fmake_variable_buffer_local (Qdisplay_fill_column_indicator_character);
+
   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;