From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Evgeny Zajcev Newsgroups: gmane.emacs.devel Subject: Re: Hollow cursor under images Date: Tue, 28 Jan 2020 11:46:15 +0300 Message-ID: References: <83bm2qea01.fsf@gnu.org> <20190304223605.GA22198@breton.holly.idiocy.org> <83k15coil1.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="0000000000006e95bd059d2f42db" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="58687"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Alan Third , emacs-devel To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Jan 28 09:47:30 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1iwMWv-000FEw-OC for ged-emacs-devel@m.gmane-mx.org; Tue, 28 Jan 2020 09:47:29 +0100 Original-Received: from localhost ([::1]:55498 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iwMWu-0007Xh-Tb for ged-emacs-devel@m.gmane-mx.org; Tue, 28 Jan 2020 03:47:28 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:52973) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iwMW4-0006YU-33 for emacs-devel@gnu.org; Tue, 28 Jan 2020 03:46:37 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iwMW2-0000Zb-Il for emacs-devel@gnu.org; Tue, 28 Jan 2020 03:46:35 -0500 Original-Received: from mail-lj1-x230.google.com ([2a00:1450:4864:20::230]:42838) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iwMVy-0000Hl-0a; Tue, 28 Jan 2020 03:46:30 -0500 Original-Received: by mail-lj1-x230.google.com with SMTP id y4so13804801ljj.9; Tue, 28 Jan 2020 00:46:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Bt/SrysL2MvNy/ZaDfuhLxNyZ/5Koken3eFUApKvRRs=; b=qhA4Faf0+1gRLTXjEiQc9WU9TofAfio9LmtAY9awweVp/OcUVla4HVRWKTWcIfgAiT VVk4dIw7biAwlN3dN5+qUQQkf7d9qCCQYe3M5wHEUWO9sFd9SGboNcBgBf6g/K3ydd81 sdaFoN1es6MSi7vplj8gFd2hDRnIrgMNHnM+/Aev/fdNE6lXCDqIp+XzwPVgDj28e0Vu jRZaxsj1lPm8F1RGOF6lWbZFSy+zKBjX1tm0oBuuPeUhTJSKzZsy1vDu3wNZFrTHL9h+ y6/OIGXIb7KMBRzOGCISjc0lh2oVwh4iFlAYo2LQdo0d/NRYnrzGA+742qUU2nhhJyAJ Xx4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Bt/SrysL2MvNy/ZaDfuhLxNyZ/5Koken3eFUApKvRRs=; b=Ln0ldhDS1QLXNQAx0aRd+F/4d6pf5da4u9v0uYyN4N+x5u/OWRAIXt18Fe/wqA6OS9 dku1gPNV30a455HHH2rq14AksXGHCdqixUQearjLZga9q31KN9VzVy9URdUhhuxkLTrw gDXorpFzacEzeL6TXlCyAxGY7TJTn9dXqw+hA2gQ0upzxTEAZtfyl4N+SEGVg0yOGaqr dT8DBoLzAnHYiXizp9KsP9luI0gKdGANuENCFTEk7lTDRMdP0zhCKbxRZ3eSpZ1RcZVj /rCrrZ3+5HWY0l8I/hNV34RQlqo+zYUudHxBF33yzFLID+xCgvYh+Ovq7k9/gceWuVi0 nWpQ== X-Gm-Message-State: APjAAAXQ3WskR4r1+lrfTC0eQgACjmvJHXGJDRh7IrbxJiY6Nl2Y6M6o D28Bpq6iejEHsnOAZ/1METRhhNm2kvF9LGOQNeLzLQ== X-Google-Smtp-Source: APXvYqzMeIKzXMPzLLjMaIdlyc4JUmbfURO8ZQgreATOLU2E6QeTFKnoSVmsUywR97/BLJg2gbQhJ4Z2XkGLtgHyh0Y= X-Received: by 2002:a2e:8e2a:: with SMTP id r10mr3897317ljk.219.1580201187119; Tue, 28 Jan 2020 00:46:27 -0800 (PST) In-Reply-To: <83k15coil1.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::230 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 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-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:244685 Archived-At: --0000000000006e95bd059d2f42db Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable =D0=BF=D0=BD, 27 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3. =D0=B2 21:33, Eli Zaretsk= ii : > > From: Evgeny Zajcev > > Date: Mon, 27 Jan 2020 15:54:56 +0300 > > Cc: Eli Zaretskii , emacs-devel > > > > This patch implements (box . WIDTH) cursor type, allowing user to defin= e > his own notion for "Large" image > > Thanks. This needs a NEWS entry and also an update for the ELisp > manual. See also a few comments below. > > > From 6406d198e9f3f5b2d93987ed0fd4fe22bd590624 Mon Sep 17 00:00:00 2001 > > From: Zajcev Evgeny > > Date: Mon, 27 Jan 2020 15:49:46 +0300 > > Subject: [PATCH] * Support for (box . WIDTH) `cursor-type' > > > > Before this commit, block cursor becomes hollow under "large" masked > > images. Notion for "large" was hardcoded to be image larger then > > 32x32 in any dimension. > > > > This patch allows user to define his own notion for "large" image, > > taking full control of block cursor look under masked images. > > > > With cursor-type equal to `box' cursor will always be block under > > masked images. This differs from former behavior for box cursor. > > To get former behavior, set `cursor-type' to (box . 32) > > Please include in the log message a ChangeLog-style list of files and > functions where you are making changes. > > > + (box . WIDTH) display a filled box cursor, but make it > > + hollow if cursor is under masked image larger then > > + WIDTH in either dimention. > ^^^^^^^^^ > "dimension" > > Also, please make it clear that WIDTH is measured in pixels. > > > +Except for (box . WIDTH) case. > > This is not a complete sentence, please clarify it. > > > + if (CONSP (arg) > > + && EQ (XCAR (arg), Qbox) > > + && RANGED_FIXNUMP (0, XCDR (arg), INT_MAX)) > > + { > > + *width =3D XFIXNUM (XCDR (arg)); > > This calls XFIXNUM no less than 3 times. I wonder if we could tweak > the code to do that only once. > > Just for my info. Should not such tweaks be done by compiler optimizer? No value is declared as volatile, it is pretty clear that there is no need to call XFIXNUM 3 times. It is written in sources just for the readability= . I'm afraid tweaking things in such way in source code will make code look ugly and less obvious > + return FILLED_BOX_CURSOR; > > + } > > + > > if (EQ (arg, Qhollow)) > > return HOLLOW_BOX_CURSOR; > > If the condition above is not fulfilled, you then make the code do 5 > more gratuitous comparisons, before it falls back on > HOLLOW_BOX_CURSOR. It is much better to return HOLLOW_BOX_CURSOR > right away if the conditions for FILLED_BOX_CURSOR weren't satisfied, > it makes the code which implements this feature much easier to read > and understand. > --=20 lg --0000000000006e95bd059d2f42db Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
=D0=BF=D0=BD, 27 =D1=8F=D0=BD=D0=B2. 2020= =D0=B3. =D0=B2 21:33, Eli Zaretskii <el= iz@gnu.org>:
> From: Evgeny Zajcev <lg.zevlg@gmail.com>
> Date: Mon, 27 Jan 2020 15:54:56 +0300
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel <emacs-devel@gnu.org>
>
> This patch implements (box . WIDTH) cursor type, allowing user to defi= ne his own notion for "Large" image

Thanks.=C2=A0 This needs a NEWS entry and also an update for the ELisp
manual.=C2=A0 See also a few comments below.

> From 6406d198e9f3f5b2d93987ed0fd4fe22bd590624 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 . WIDTH) `cursor-type'
>
> Before this commit, block cursor becomes hollow under "large"= ; masked
> images.=C2=A0 Notion for "large" was hardcoded to be image l= arger then
> 32x32 in any dimension.
>
> This patch allows user to define his own notion for "large" = image,
> taking full control of block cursor look under masked images.
>
> With cursor-type equal to `box' cursor will always be block under<= br> > masked images.=C2=A0 This differs from former behavior for box cursor.=
> To get former behavior, set `cursor-type' to (box . 32)

Please include in the log message a ChangeLog-style list of files and
functions where you are making changes.

> +=C2=A0 (box . WIDTH)=C2=A0 =C2=A0display a filled box cursor, but mak= e it
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hollow= if cursor is under masked image larger then
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 WIDTH = in either dimention.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^^^^^^^^^
"dimension"

Also, please make it clear that WIDTH is measured in pixels.

> +Except for (box . WIDTH) case.

This is not a complete sentence, please clarify it.

> +=C2=A0 if (CONSP (arg)
> +=C2=A0 =C2=A0 =C2=A0 && EQ (XCAR (arg), Qbox)
> +=C2=A0 =C2=A0 =C2=A0 && RANGED_FIXNUMP (0, XCDR (arg), INT_MA= X))
> +=C2=A0 =C2=A0 {
> +=C2=A0 =C2=A0 =C2=A0 *width =3D XFIXNUM (XCDR (arg));

This calls XFIXNUM no less than 3 times.=C2=A0 I wonder if we could tweak the code to do that only once.


Just for my info.=C2=A0 Should not suc= h tweaks be done by compiler optimizer?=C2=A0 No value is declared as volat= ile, it is pretty clear that there is no need to call XFIXNUM 3 times.=C2= =A0 It is written in sources just for the readability.

=
I'm afraid tweaking things in such way in source code will make co= de look ugly and less obvious

> +=C2=A0 =C2=A0 =C2=A0 return FILLED_BOX_CURSOR;
> +=C2=A0 =C2=A0 }
> +
>=C2=A0 =C2=A0 if (EQ (arg, Qhollow))
>=C2=A0 =C2=A0 =C2=A0 return HOLLOW_BOX_CURSOR;

If the condition above is not fulfilled, you then make the code do 5
more gratuitous comparisons, before it falls back on
HOLLOW_BOX_CURSOR.=C2=A0 It is much better to return HOLLOW_BOX_CURSOR
right away if the conditions for FILLED_BOX_CURSOR weren't satisfied, it makes the code which implements this feature much easier to read
and understand.


--
lg
--0000000000006e95bd059d2f42db--