From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Add support for `ch' and `cw' dimension specifiers for the image Date: Thu, 28 Mar 2024 12:06:13 +0200 Message-ID: <86cyrehdre.fsf@gnu.org> References: <86sf0j1q0c.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="36012"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Evgeny Zajcev , Alan Third Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Mar 28 11:07:36 2024 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 1rpmfX-0009C9-Sr for ged-emacs-devel@m.gmane-mx.org; Thu, 28 Mar 2024 11:07:36 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rpmeq-0004Gs-At; Thu, 28 Mar 2024 06:06:52 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rpmek-0004GC-2e for emacs-devel@gnu.org; Thu, 28 Mar 2024 06:06:49 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rpmed-0004aU-H4; Thu, 28 Mar 2024 06:06:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=4PrFPP8lSJuOk6iONi7szYm/ngoNRjC/EtdKWIu+TTk=; b=OFuwUmiBPRMM6bYFM6CL NtueSaXRpBDTtiq9GChM2SpmNNb1fCvk4Kityzxt2gfX6q436PBj7s81tlxtHbAaPX0rseC4hi43b kVLdjhq3n7xK/gygqYmtyErnCPk639RcDXSm97nTK+qidH0fxEo0+m7of9G9VYUoOroDNUOYHRvpU YoVPmJwfbzcMGxkQ0QPtOsh9EzhP691wtijQiu4lMRohbn+Pe5NQ3TdfGn4IpujMZzwZwJZ70T2MS gSnKAjo8K8zeOqTV9SPTXEeLHAccPtK+cJ1Ms5RLtqFCk5uCD42xcPHXXybEOuVQG+AtOOm6W0I7T CTcHJCrbUw6GnA==; In-Reply-To: (message from Evgeny Zajcev on Thu, 21 Mar 2024 22:14:51 +0300) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 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-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:317353 Archived-At: > From: Evgeny Zajcev > Date: Thu, 21 Mar 2024 22:14:51 +0300 > Cc: emacs-devel@gnu.org > > чт, 21 мар. 2024 г. в 19:57, Eli Zaretskii : > > > From: Evgeny Zajcev > > Date: Thu, 21 Mar 2024 17:53:09 +0300 > > > > With applied patch and image specified as: > > > > (list 'image :type 'svg :file "file.svg" :scale 1.0 :ascent 'center > > :width '(2 . cw) > > :max-height '(1 . ch)) > > ENOPATCH > > :)) sorry, here it is Thanks. Alan, could you please take a look and comment on this? I have a couple of minor stylistic comments below. > diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi > index 4dbb4afb20d..73671a21e7f 100644 > --- a/doc/lispref/display.texi > +++ b/doc/lispref/display.texi > @@ -5788,8 +5788,11 @@ Image Descriptors > length in @dfn{ems}@footnote{In typography an em is a distance > equivalent to the height of the type. For example when using 12 point > type 1 em is equal to 12 points. Its use ensures distances and type > -remain proportional.}. One em is equivalent to the height of the font > -and @var{value} may be an integer or a float. > +remain proportional.}. One em is equivalent to the size of the font > +and @var{value} may be an integer or a float. Also, dimension can be Here, you changed the description of "em" from "height of the font" to "size of the font". Is this intentional, and if so, why it is better to say "size" here? > + /* Details of the font used to calculate image size relative to the > + canonical character size, with `ch' and `cw' specifiers. */ ^^ Please leave two spaces after the last sentence of the comment. > + if (CONSP (value) && NUMBERP (CAR (value))) > + { > + if (EQ (Qem, CDR (value))) > + return scale_image_size (img->face_font_size, > + 1, XFLOATINT (CAR (value))); > + if (EQ (Qch, CDR (value))) > + return scale_image_size (img->face_font_height, > + 1, XFLOATINT (CAR (value))); > + if (EQ (Qcw, CDR (value))) > + return scale_image_size (img->face_font_width, > + 1, XFLOATINT (CAR (value))); Minor efficiency comment: it is better to compute CDR(value) just once and store it in a temporary: if (CONSP (value) && NUMBERP (CAR (value))) { Lisp_Object dim = CDR (value); if (EQ (Qem, dim)) return scale_image_size (img->face_font_size, 1, XFLOATINT (CAR (value))); if (EQ (Qch, dim)) return scale_image_size (img->face_font_height, 1, XFLOATINT (CAR (value))); etc. (Optimizing compilers will do this automatically, but an unoptimized build might become a tad faster.) Finally, please include a ChangeLog-style commit log message for this patch; see CONTRIBUTE for how we expect that to be formatted (and you can use "git log" to see what we do in practice).