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: move_it_vertically_backward question Date: Thu, 23 Dec 2021 11:49:11 +0200 Message-ID: <83wnjvveg8.fsf@gnu.org> References: <87lf0pw78r.fsf.ref@yahoo.com> <83tuf9gdg5.fsf@gnu.org> <87pmpwkikp.fsf@yahoo.com> <83mtl0hnm5.fsf@gnu.org> <87czlwkfpk.fsf@yahoo.com> <8735mskal2.fsf@yahoo.com> <83ee6che8h.fsf@gnu.org> <87y24kisgf.fsf@yahoo.com> <838rwkhcqb.fsf@gnu.org> <87r1achulq.fsf@yahoo.com> <83pmpub41v.fsf@gnu.org> <87pmptbeii.fsf@yahoo.com> <8335mp9evn.fsf@gnu.org> <877dc19cpc.fsf@yahoo.com> <83zgox7xmf.fsf@gnu.org> <87ilvk99j0.fsf@yahoo.com> <83lf0g79je.fsf@gnu.org> <87mtkw5bn4.fsf@yahoo.com> <871r26w1v4.fsf@yahoo.com> <83czlpzy2n.fsf@gnu.org> <87wnjxv4ye.fsf@yahoo.com> <87h7b0m7k5.fsf@yahoo.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="15105"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Po Lu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Dec 23 10:49:51 2021 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 1n0KjO-0003jy-Fx for ged-emacs-devel@m.gmane-mx.org; Thu, 23 Dec 2021 10:49:50 +0100 Original-Received: from localhost ([::1]:49146 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n0KjM-0003YW-SL for ged-emacs-devel@m.gmane-mx.org; Thu, 23 Dec 2021 04:49:48 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:58954) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n0Kih-0002tq-3X for emacs-devel@gnu.org; Thu, 23 Dec 2021 04:49:07 -0500 Original-Received: from [2001:470:142:3::e] (port=49566 helo=fencepost.gnu.org) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n0Kig-0002Na-QP; Thu, 23 Dec 2021 04:49:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=mVUJC9TaCD+RqxRVPxfkr9mrPHvrKdrc5dcu+aClzn0=; b=ig1t5gylnpys usxKkQj3QPu90mlOgVz93I+wiIIgTFLMfunAtxbacRnmb9Nz5Cud56Xztkue7ddHYFUUlspNUxwc0 y98VDLPzFfB2BvkMUeHprUNMsTitM/v+N4iohkFDnn7WTWE32s6W+RyGob2CQDT9qUsEvdBS8WUYC t8FVf9XECfuWWX60lYEr/OQ/5gbrhf8K2f9ShS9UchWJQ4cONbSQ8SG8S30hqS26ITVdKu8U826b/ uBRlVbq4X8QDj+TaGuQqyYdBmUIviWmOx/diw7JWLyNM+8Ev8AXid+caDGt1014I8G9z8iRWGo8xu 6dfSt9Ce/7iDR8k+flg4FA==; Original-Received: from [87.69.77.57] (port=3919 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n0Kig-0001Ma-M5; Thu, 23 Dec 2021 04:49:06 -0500 In-Reply-To: <87h7b0m7k5.fsf@yahoo.com> (message from Po Lu on Thu, 23 Dec 2021 09:30:34 +0800) 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" Xref: news.gmane.io gmane.emacs.devel:282899 Archived-At: > From: Po Lu > Cc: emacs-devel@gnu.org > Date: Thu, 23 Dec 2021 09:30:34 +0800 > > > Also, here's a patch with some of the changes made. I tried with the > > documentation as well, but I'm pretty bad with that stuff. > > I tried updating the documentation again to better fit what's actually > done (including documenting that measurements always begin at the > beginning of the visual line). > > Please take a look and see if it's OK to install this now, thanks. Does the code DTRT in your use cases, and is it OK performance-wise to consider it stable enough for master? > * src/xdisp.c (window_text_pixel_size): Understand a special > format of `from' that specifies the amount of pixels above a > position. ^^^^^ "above or below". Or maybe just "at a given pixel offset from". > diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi > index 98a15404f9..7e6ee59824 100644 > --- a/doc/lispref/display.texi > +++ b/doc/lispref/display.texi > @@ -2090,19 +2090,23 @@ Size of Displayed Text > This function returns the size of the text of @var{window}'s buffer in > pixels. @var{window} must be a live window and defaults to the > selected one. The return value is a cons of the maximum pixel-width > -of any text line and the maximum pixel-height of all text lines. This > -function exists to allow Lisp programs to adjust the dimensions of > -@var{window} to the buffer text it needs to display. > +of any text line and the maximum pixel-height of all text lines, or if > +@var{from} is a cons, a list of the pixel-width, pixel-height, and the > +buffer position of the first line that was measured. This sentence now becomes too long and thus confusing. I suggest to leave the original sentence alone, and add a new sentence, something like The return value can also optionally (see below) include the buffer position of the first line whose dimensions were measured. > This function > +exists to allow Lisp programs to adjust the dimensions of @var{window} > +to the buffer text it needs to display. I would add "and for other similar situations" to this sentence. > +character. If @var{from} is a cons, the cdr specifies a position, and > +the car specifies the minimum amount of pixels above or below the > +beginning of the visual line at that position to start measuring from. First, I think you swapped the car and cdr. Second, I would suggest to simplify/clarify: ... its @code{car} specifies a buffer position, and its @code{car} specifies the vertical offset in pixels from that position to the first screen line whose text is to be measured. (The measurement will start from the visual beginning of that screen line.) And finally, please add here the detailed description of the return value, both when FROM is a position and when it's a cons cell. Since the forms of FROM are already described, such a description of the return value here will be less confusing. > ++++ > +** 'window-text-pixel-size' can now measure from a set amount of pixels above a position. This is too long for a NEWS heading. Can you make it shorter, leaving the details for the body? I would also add more explanations, because this sounds very vague. > + else if (CONSP (from)) > + { > + start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV); > + bpos = CHAR_TO_BYTE (start); > + CHECK_FIXNUM (XCDR (from)); > + vertical_offset = XFIXNUM (XCDR (from)); In principle, the offset could be a bignum or even a float, so the restriction here seems to be too stringent, no? A float in particular could be useful, I think, in case the caller calculated the pixels as a float. > + if (vertical_offset != 0) > + { > + int last_y; > + it.current_y = 0; > + > + move_it_by_lines (&it, 0); > + > + if (vertical_offset < 0) > + { > + while (it.current_y > vertical_offset) > + { > + last_y = it.current_y; > + move_it_vertically_backward (&it, > + (abs (vertical_offset) > + + it.current_y)); > + > + if (it.current_y == last_y) > + break; > + } > + } > + else > + { > + move_it_vertically (&it, vertical_offset); > + } > + Can you add a comment explaining why in the negative case you call move_it_vertically_backward in a loop, while in the positive case you call move_it_vertically just once? > DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0, > - doc: /* Return the size of the text of WINDOW's buffer in pixels. > -WINDOW must be a live window and defaults to the selected one. The > -return value is a cons of the maximum pixel-width of any text line > -and the pixel-height of all the text lines in the accessible portion > -of buffer text. > + doc: /* Return the size of the text of WINDOW's buffer in > +pixels. WINDOW must be a live window and defaults to the selected The first line must be a complete sentence, so your reformatting of it is a step backward. > +one. The return value is a cons of the maximum pixel-width of any > +text line and the pixel-height of all the text lines in the accessible > +portion of buffer text or a list of the maximum pixel-width, > +pixel-height, and the buffer position of the line at FROM. Here, too, I would suggest to first say what is the form of the return value in the "usual" case, and then add a separate sentence like If FROM is a cons cell, the return value includes, in addition to the dimensions, also a third element that provides the buffer position from which measuring of the text dimensions was actually started. > +of the buffer. If FROM is a cons, the cdr specifies the amount of > +pixels above or below a buffer position to begin measuring text, and > +the car specifies the buffer position. Consider rewording like I suggested for the manual. > In that case, a list is > +returned. This was already said above, so it's redundant. Thanks.