bug#37858: 27.0.50; Ensure a minimum width for `space` display prop

bug#37858: 27.0.50; Ensure a minimum width for `space` display prop

[Stefan Monnier]

Package: Emacs
Version: 27.0.50


For text displayed in columns, alignment is generally obtained with
a `display` text-property of the form

    (space :align-to FOO)

This works great when the previous text ends before FOO, but when we
mis-calculate (or didn't calculate at all) and the previous text already
extends further than the desired alignment of the following text, such
space is reduced down to 0 pixels which is often not what we want.

Sometimes one can workaround this by placing 2 spaces in the buffer: one
with the :align-to and another fixed size space.  But it can be
cumbersome to do that and it leads to undesirable artifacts (e.g. the
cursor can be placed between the two space).

So, I'd like to extend our `space` specifications so as to be able to
specify a minimum width.  I came up with the patch below which lets you
write:

    (space :align-to FOO :min-width BAR)

which seems to work fine, but while trying to update the Elisp doc for
it I realized that maybe a better option is to extend the acceptable
forms for FOO so it can be of the form:

    (space :align-to (max FOO (+ BAR current-x)))

WDYT?


        Stefan


diff --git a/src/xdisp.c b/src/xdisp.c
index be1c209553..fd825d2dfe 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -29218,6 +29218,28 @@ append_stretch_glyph (struct it *it, Lisp_Object object,
 
 #endif	/* HAVE_WINDOW_SYSTEM */
 
+static int
+compute_relative_width (struct it *it, Lisp_Object prop)
+{
+  struct it it2;
+  unsigned char *p = BYTE_POS_ADDR (IT_BYTEPOS (*it));
+
+  it2 = *it;
+  if (it->multibyte_p)
+    it2.c = it2.char_to_display = STRING_CHAR_AND_LENGTH (p, it2.len);
+  else
+    {
+      it2.c = it2.char_to_display = *p, it2.len = 1;
+      if (! ASCII_CHAR_P (it2.c))
+	it2.char_to_display = BYTE8_TO_CHAR (it2.c);
+    }
+
+  it2.glyph_row = NULL;
+  it2.what = IT_CHARACTER;
+  PRODUCE_GLYPHS (&it2);
+  return NUMVAL (prop) * it2.pixel_width;
+}
+
 /* Produce a stretch glyph for iterator IT.  IT->object is the value
    of the glyph property displayed.  The value must be a list
    `(space KEYWORD VALUE ...)' with the following KEYWORD/VALUE pairs
@@ -29288,23 +29310,7 @@ produce_stretch_glyph (struct it *it)
       /* Relative width `:relative-width FACTOR' specified and valid.
 	 Compute the width of the characters having the `glyph'
 	 property.  */
-      struct it it2;
-      unsigned char *p = BYTE_POS_ADDR (IT_BYTEPOS (*it));
-
-      it2 = *it;
-      if (it->multibyte_p)
-	it2.c = it2.char_to_display = STRING_CHAR_AND_LENGTH (p, it2.len);
-      else
-	{
-	  it2.c = it2.char_to_display = *p, it2.len = 1;
-	  if (! ASCII_CHAR_P (it2.c))
-	    it2.char_to_display = BYTE8_TO_CHAR (it2.c);
-	}
-
-      it2.glyph_row = NULL;
-      it2.what = IT_CHARACTER;
-      PRODUCE_GLYPHS (&it2);
-      width = NUMVAL (prop) * it2.pixel_width;
+      width = compute_relative_width (it, prop);
     }
   else if ((prop = Fplist_get (plist, QCalign_to), !NILP (prop))
 	   && calc_pixel_width_or_height (&tem, it, prop, font, true,
@@ -29323,6 +29329,21 @@ produce_stretch_glyph (struct it *it)
     /* Nothing specified -> width defaults to canonical char width.  */
     width = FRAME_COLUMN_WIDTH (it->f);
 
+  if ((prop = Fplist_get (plist, QCmin_width), !NILP (prop))
+      && calc_pixel_width_or_height (&tem, it, prop, font, true, 0))
+    {
+      /* Absolute minimum width `:min-width WIDTH' specified and valid.  */
+      if (width < tem)
+        width = tem;
+    }
+  else if (prop = Fplist_get (plist, QCmin_relative_width), NUMVAL (prop) > 0)
+    {
+      /* Relative width `:min-relative-width FACTOR' specified and valid.  */
+      int tem = compute_relative_width (it, prop);
+      if (width < tem)
+        width = tem;
+    }
+
   if (width <= 0 && (width < 0 || !zero_width_ok_p))
     width = 1;
 
@@ -34275,6 +34296,8 @@ syms_of_xdisp (void)
   DEFSYM (QCalign_to, ":align-to");
   DEFSYM (QCrelative_width, ":relative-width");
   DEFSYM (QCrelative_height, ":relative-height");
+  DEFSYM (QCmin_relative_width, ":min-relative-width");
+  DEFSYM (QCmin_width, ":min-width");
   DEFSYM (QCeval, ":eval");
   DEFSYM (QCpropertize, ":propertize");
   DEFSYM (QCfile, ":file");

bug#37858: 27.0.50; Ensure a minimum width for `space` display prop

[Robert Pluim]

>>>>> On Mon, 21 Oct 2019 16:03:58 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    Stefan> Package: Emacs
    Stefan> Version: 27.0.50


    Stefan> For text displayed in columns, alignment is generally obtained with
    Stefan> a `display` text-property of the form

    Stefan>     (space :align-to FOO)

    Stefan> This works great when the previous text ends before FOO, but when we
    Stefan> mis-calculate (or didn't calculate at all) and the previous text already
    Stefan> extends further than the desired alignment of the following text, such
    Stefan> space is reduced down to 0 pixels which is often not what we want.

    Stefan> Sometimes one can workaround this by placing 2 spaces in the buffer: one
    Stefan> with the :align-to and another fixed size space.  But it can be
    Stefan> cumbersome to do that and it leads to undesirable artifacts (e.g. the
    Stefan> cursor can be placed between the two space).

    Stefan> So, I'd like to extend our `space` specifications so as to be able to
    Stefan> specify a minimum width.  I came up with the patch below which lets you
    Stefan> write:

    Stefan>     (space :align-to FOO :min-width BAR)

    Stefan> which seems to work fine, but while trying to update the Elisp doc for
    Stefan> it I realized that maybe a better option is to extend the acceptable
    Stefan> forms for FOO so it can be of the form:

    Stefan>     (space :align-to (max FOO (+ BAR current-x)))

Hmm, I think I probably prefer the former. Perhaps when you've
documented the semantics of your :min-relative-width addition Iʼll be
able to judge better :-)

Robert

bug#37858: 27.0.50; Ensure a minimum width for `space` display prop

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 21 Oct 2019 16:03:58 -0400
> 
> So, I'd like to extend our `space` specifications so as to be able to
> specify a minimum width.  I came up with the patch below which lets you
> write:
> 
>     (space :align-to FOO :min-width BAR)
> 
> which seems to work fine, but while trying to update the Elisp doc for
> it I realized that maybe a better option is to extend the acceptable
> forms for FOO so it can be of the form:
> 
>     (space :align-to (max FOO (+ BAR current-x)))

Since :align-to already supports expressions of the forms described in
the node "Pixel Specification", to have the latter you'd need:

  . implement a new OP called 'max' (and probably also 'min' for a
    good measure);
  . implement a new POS called, say, 'beg', which will evaluate to the
    x coordinate of where the space display property begins

and then you will have your feature for free, I think.

bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Stefan Kangas]

When I browse Info in "emacs -Q", the default formatting is fine when
I say C-h i and I see entries nicely lined up like so:

* IDLWAVE           Major mode and shell for IDL files.
* Newsticker        A feed reader for Emacs.

However, when I increase the font size using C-x C-+, I end up with
text that looks visually more like this:

* IDLWAVEMajor mode and shell for IDL files.
* NewstickerA feed reader for Emacs.

I see the same in menu entries in individual nodes.

I didn't look into it, but it would be nice if the formatting of menu
items could adjust as the font size increases.

Best regards,
Stefan Kangas

bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 23 Oct 2019 03:43:55 +0200
> 
> When I browse Info in "emacs -Q", the default formatting is fine when
> I say C-h i and I see entries nicely lined up like so:
> 
> * IDLWAVE           Major mode and shell for IDL files.
> * Newsticker        A feed reader for Emacs.
> 
> However, when I increase the font size using C-x C-+, I end up with
> text that looks visually more like this:
> 
> * IDLWAVEMajor mode and shell for IDL files.
> * NewstickerA feed reader for Emacs.

That's :align-to display property in action.  See bug#37858 for one
attempt to enhance it.

bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> So, I'd like to extend our `space` specifications so as to be able to
> specify a minimum width.  I came up with the patch below which lets you
> write:
>
>     (space :align-to FOO :min-width BAR)
>
> which seems to work fine, but while trying to update the Elisp doc for
> it I realized that maybe a better option is to extend the acceptable
> forms for FOO so it can be of the form:
>
>     (space :align-to (max FOO (+ BAR current-x)))

I think I'd actually prefer the first form -- it's easy to reason about,
and does what most usage cases want (i.e., align if possible, but if
not, then at least leave some space so that things don't run into each
other).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen [2022-05-07 14:07:24] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> So, I'd like to extend our `space` specifications so as to be able to
>> specify a minimum width.  I came up with the patch below which lets you
>> write:
>>
>>     (space :align-to FOO :min-width BAR)
>>
>> which seems to work fine, but while trying to update the Elisp doc for
>> it I realized that maybe a better option is to extend the acceptable
>> forms for FOO so it can be of the form:
>>
>>     (space :align-to (max FOO (+ BAR current-x)))
>
> I think I'd actually prefer the first form -- it's easy to reason about,
> and does what most usage cases want (i.e., align if possible, but if
> not, then at least leave some space so that things don't run into each
> other).

The patch I came up with back then doesn't work right.  IIRC it's
because we need to change both the redisplay code and the
`current-column` code and it only changed one of the two.

IIRC, I decided then that the right fix is to rewrite the
`current-column` code to use the redisplay code (instead of trying to
mimic it), but I didn't get around to that (and IIRC it's not
completely straightforward because `current-column` currently behaves
differently *on purpose* in some cases (most importantly w.r.t treating
ellipsis-erased text) so fixing it right will imply changes in behavior
and figuring out how to do it without breaking existing uses).


        Stefan

bug#37858: bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Eli Zaretskii]

> Cc: 37880@debbugs.gnu.org, 37858@debbugs.gnu.org
> Date: Sat, 07 May 2022 10:29:54 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> The patch I came up with back then doesn't work right.  IIRC it's
> because we need to change both the redisplay code and the
> `current-column` code and it only changed one of the two.
> 
> IIRC, I decided then that the right fix is to rewrite the
> `current-column` code to use the redisplay code (instead of trying to
> mimic it), but I didn't get around to that (and IIRC it's not
> completely straightforward because `current-column` currently behaves
> differently *on purpose* in some cases (most importantly w.r.t treating
> ellipsis-erased text) so fixing it right will imply changes in behavior
> and figuring out how to do it without breaking existing uses).

When did you last look at what current-column does in the context of
the issue being discussed here?

I'm not sure it does what you had in mind, but some changes were done
there recently, and the behavior in similar contexts did change.

bug#37858: bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii [2022-05-07 17:38:10] wrote:
> When did you last look at what current-column does in the context of
> the issue being discussed here?

It was soon after I posted this bug report.

> I'm not sure it does what you had in mind, but some changes were done
> there recently, and the behavior in similar contexts did change.

I haven't looked at recent changes in this area, so maybe things would
be a bit easier now, tho looking at `scan_for_column` in `master`,
I still see that we re-implement in `check_display_width` (part of) the
code used in `xdisp.c` to handle the display property, so AFAICT the
duplication is still present.


        Stefan

bug#37880: bug#37858: bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: larsi@gnus.org,  37880@debbugs.gnu.org,  37858@debbugs.gnu.org
> Date: Sat, 07 May 2022 11:58:41 -0400
> 
> I haven't looked at recent changes in this area, so maybe things would
> be a bit easier now, tho looking at `scan_for_column` in `master`,
> I still see that we re-implement in `check_display_width` (part of) the
> code used in `xdisp.c` to handle the display property, so AFAICT the
> duplication is still present.

There's no duplication.  We simply use the same infrastructure as the
display code does.

bug#37858: bug#37880: 27.0.50; Changing font size in Info-mode messes up formatting

[Eli Zaretskii]

> Cc: larsi@gnus.org, 37880@debbugs.gnu.org, 37858@debbugs.gnu.org
> Date: Sat, 07 May 2022 19:06:55 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: larsi@gnus.org,  37880@debbugs.gnu.org,  37858@debbugs.gnu.org
> > Date: Sat, 07 May 2022 11:58:41 -0400
> > 
> > I haven't looked at recent changes in this area, so maybe things would
> > be a bit easier now, tho looking at `scan_for_column` in `master`,
> > I still see that we re-implement in `check_display_width` (part of) the
> > code used in `xdisp.c` to handle the display property, so AFAICT the
> > duplication is still present.
> 
> There's no duplication.  We simply use the same infrastructure as the
> display code does.

And, btw, I don't think I agree with your assertion that we should use
the display routines for current-column and friends.  The
column-oriented functions count actual (not canonical) columns,
whereas display code counts pixels.  It makes little sense to me to
call the display code, which will load fonts, call jit-lock, and
whatnot, just to count columns like current-column expects.