Re: Hollow cursor under images

[Evgeny Zajcev <lg.zevlg@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2393 bytes --]

вт, 28 янв. 2020 г. в 21:22, Eli Zaretskii <eliz@gnu.org>:

> > From: Evgeny Zajcev <lg.zevlg@gmail.com>
> > Date: Tue, 28 Jan 2020 14:55:39 +0300
> > Cc: Alan Third <alan@idiocy.org>, emacs-devel <emacs-devel@gnu.org>
> >
> > I tried to fulfill all the review comments in this updated patch
>
> Thanks.  A few minor comments below.
>
> > * buffer.c (cursor-type): Add commentary about (box . WIDTH)
> >   cursor-type
>
> End this sentence with a period.
>
> > * xdisp.c (get_specified_cursor_type): Check for `cursor-type' in form
> >   (box . WIDTH)
>
> Likewise.
>
> > * xdisp.c (get_window_cursor_type): Check masked image size for
> >   (box . WIDTH) cursor-type.
>
> Changes in several functions in the same file should mention the file
> name just once.  Like this:
>
>   * xdisp.c (get_specified_cursor_type): Check for `cursor-type' in form
>   '(box . WIDTH)'.
>   ((get_window_cursor_type): Check masked image size for(box . WIDTH)
>   cursor-type.
>
> > * doc/emacs/display.texi, doc/emacs/display.texi: Add description
> >   for (box . WIDTH) `cursor-type'
> >
> > * etc/NEWS: Add note about (box . WIDTH) `cursor-type'
>
> Period missing at end of each sentence.
>
> > +@code{(box . @var{width})} (box cursor becoming a hollow box under
> > +masked images larger than @var{width} in either dimension),
>
> I think it's better to use "size", not 'width" here, because "width"
> is closely associated with only one dimension, and you mean both.  Or
> maybe even "min-size", for better mnemonic value.
>
>
Yes! I like "size" name too


> > +** Support for `(box . WIDTH)' cursor-type.  By default, `box' cursor
> > +always has a filled box shape.  Unless you specify cursor-type to be
> > +`(box . WIDTH)'.  In such case, cursor becomes a hollow box if the
> > +point is under masked image larger than `WIDTH' in any dimension.
>
> The first sentence of a NEWS entry should be alone on the header
> line.  That's because NEWS is visited in Outline mode, where users can
> hide the text of the entry, leaving just the headings.  So leave only
> the first sentence on the heading line, and move all the rest to the
> body starting from the next line.
>
> Also, we prefer quoting 'like this' in NEWS (and in log messages,
> btw).
>

Here is the updated version, tried to fix all the formatting issues.

-- 
lg

[-- Attachment #1.2: Type: text/html, Size: 3389 bytes --]

[-- Attachment #2: 0001-Support-for-box-.-SIZE-cursor-type.patch --]
[-- Type: text/x-patch, Size: 5486 bytes --]

From be2b70bd3995f7dd333708ed4a973239b78e13cc Mon Sep 17 00:00:00 2001
From: Zajcev Evgeny <zevlg@yandex.ru>
Date: Mon, 27 Jan 2020 15:49:46 +0300
Subject: [PATCH] Support for (box . SIZE) `cursor-type'

To control minimum size of a masked image under which box cursor
becomes hollow.

* buffer.c (cursor-type): Add commentary about (box . SIZE)
  cursor-type.

* xdisp.c (get_specified_cursor_type): Check for `cursor-type' in form
  (box . SIZE).
  (get_window_cursor_type): Check masked image size for
  (box . SIZE) cursor-type.

* doc/emacs/display.texi, doc/emacs/display.texi: Add description
  for (box . SIZE) `cursor-type'.

* etc/NEWS: Add note about (box . SIZE) `cursor-type'.
---
 doc/emacs/display.texi  |  2 ++
 doc/lispref/frames.texi |  3 +++
 etc/NEWS                |  6 ++++++
 src/buffer.c            |  3 +++
 src/xdisp.c             | 28 ++++++++++++++--------------
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/doc/emacs/display.texi b/doc/emacs/display.texi
index 8444aef..dc6fe3a 100644
--- a/doc/emacs/display.texi
+++ b/doc/emacs/display.texi
@@ -1654,6 +1654,8 @@ Cursor Display
 the text shown under the cursor is drawn using the frame's background
 color.)  To change its shape, customize the buffer-local variable
 @code{cursor-type}; possible values are @code{box} (the default),
+@code{(box . @var{SIZE})} (box cursor becoming a hollow box under
+masked images larger than @var{SIZE} pixels in either dimension),
 @code{hollow} (a hollow box), @code{bar} (a vertical bar), @code{(bar
 . @var{n})} (a vertical bar @var{n} pixels wide), @code{hbar} (a
 horizontal bar), @code{(hbar . @var{n})} (a horizontal bar @var{n}
diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index 2bb505c..70ebe2e 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -2220,6 +2220,9 @@ Cursor Parameters
 @table @code
 @item box
 Display a filled box.  (This is the default.)
+@item (box . @var{SIZE})
+Display a filled box.  However, display it as a hollow box if point is
+under masked image larger than @var{SIZE} pixels in either dimension.
 @item hollow
 Display a hollow box.
 @item nil
diff --git a/etc/NEWS b/etc/NEWS
index 5395f2c..27ea895 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -64,6 +64,12 @@ It was declared obsolete in Emacs 27.1.
 \f
 * Changes in Emacs 28.1
 
+** Support for '(box . SIZE)' cursor-type.
+By default, 'box' cursor always has a filled box shape.  Unless you
+specify cursor-type to be '(box . SIZE)'.  In such case, cursor
+becomes a hollow box if the point is under masked image larger than
+'SIZE' pixels in any dimension.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/src/buffer.c b/src/buffer.c
index 5c65d4d..cc7d4e4 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -6247,6 +6247,9 @@ from (abs POSITION).  If POSITION is positive, point was at the front
   t               use the cursor specified for the frame
   nil             don't display a cursor
   box             display a filled box cursor
+  (box . SIZE)    display a filled box cursor, but make it
+                  hollow if cursor is under masked image larger than
+                  SIZE pixels in either dimension.
   hollow          display a hollow box cursor
   bar             display a vertical bar cursor with default width
   (bar . WIDTH)   display a vertical bar cursor with width WIDTH
diff --git a/src/xdisp.c b/src/xdisp.c
index 68a504f..038b8e5 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -30741,14 +30741,6 @@ get_specified_cursor_type (Lisp_Object arg, int *width)
       return BAR_CURSOR;
     }
 
-  if (CONSP (arg)
-      && EQ (XCAR (arg), Qbar)
-      && RANGED_FIXNUMP (0, XCDR (arg), INT_MAX))
-    {
-      *width = XFIXNUM (XCDR (arg));
-      return BAR_CURSOR;
-    }
-
   if (EQ (arg, Qhbar))
     {
       *width = 2;
@@ -30756,11 +30748,16 @@ get_specified_cursor_type (Lisp_Object arg, int *width)
     }
 
   if (CONSP (arg)
-      && EQ (XCAR (arg), Qhbar)
       && RANGED_FIXNUMP (0, XCDR (arg), INT_MAX))
     {
       *width = XFIXNUM (XCDR (arg));
-      return HBAR_CURSOR;
+
+      if (EQ (XCAR (arg), Qbox))
+          return FILLED_BOX_CURSOR;
+      else if (EQ (XCAR (arg), Qbar))
+          return BAR_CURSOR;
+      else if (EQ (XCAR (arg), Qhbar))
+          return HBAR_CURSOR;
     }
 
   /* Treat anything unknown as "hollow box cursor".
@@ -30898,12 +30895,15 @@ get_window_cursor_type (struct window *w, struct glyph *glyph, int *width,
 	      struct image *img = IMAGE_OPT_FROM_ID (f, glyph->u.img_id);
 	      if (img != NULL && IMAGEP (img->spec))
 		{
-		  /* Arbitrarily, interpret "Large" as >32x32 and >NxN
+		  /* Interpret "large" as >SIZExSIZE and >NxN
+                     where SIZE is the value from cursor-type in form (box . SIZE),
 		     where N = size of default frame font size.
-		     This should cover most of the "tiny" icons people may use.  */
+                     So, setting cursor-type to (box . 32) should cover most of
+                     the "tiny" icons people may use.  */
 		  if (!img->mask
-		      || img->width > max (32, WINDOW_FRAME_COLUMN_WIDTH (w))
-		      || img->height > max (32, WINDOW_FRAME_LINE_HEIGHT (w)))
+                      || (CONSP (BVAR (b, cursor_type))
+                          && img->width > max (*width, WINDOW_FRAME_COLUMN_WIDTH (w))
+                          && img->height > max (*width, WINDOW_FRAME_LINE_HEIGHT (w))))
 		    cursor_type = HOLLOW_BOX_CURSOR;
 		}
 	    }
-- 
2.7.4