From mboxrd@z Thu Jan  1 00:00:00 1970
Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail
From: Eli Zaretskii <eliz@gnu.org>
Newsgroups: gmane.emacs.bugs
Subject: bug#67275: [PATCH] ; Improve and add tests for Completion Preview mode
Date: Sun, 19 Nov 2023 12:58:01 +0200
Message-ID: <8334x2ko1y.fsf@gnu.org>
References: <m1sf529gzw.fsf@dazzs-mbp.home>
Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214";
	logging-data="16802"; mail-complaints-to="usenet@ciao.gmane.io"
Cc: 67275@debbugs.gnu.org
To: Eshel Yaron <me@eshelyaron.com>
Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Nov 19 11:59:17 2023
Return-path: <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org>
Envelope-to: geb-bug-gnu-emacs@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 <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org>)
	id 1r4fWH-0004Bf-9F
	for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 19 Nov 2023 11:59:17 +0100
Original-Received: from localhost ([::1] helo=lists1p.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.90_1)
	(envelope-from <bug-gnu-emacs-bounces@gnu.org>)
	id 1r4fW5-00005R-3t; Sun, 19 Nov 2023 05:59:05 -0500
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 <Debian-debbugs@debbugs.gnu.org>)
 id 1r4fW1-0008UE-5x
 for bug-gnu-emacs@gnu.org; Sun, 19 Nov 2023 05:59:01 -0500
Original-Received: from debbugs.gnu.org ([2001:470:142:5::43])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)
 (Exim 4.90_1) (envelope-from <Debian-debbugs@debbugs.gnu.org>)
 id 1r4fW0-000580-My
 for bug-gnu-emacs@gnu.org; Sun, 19 Nov 2023 05:59:00 -0500
Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2)
 (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1r4fW2-0007ia-13
 for bug-gnu-emacs@gnu.org; Sun, 19 Nov 2023 05:59:02 -0500
X-Loop: help-debbugs@gnu.org
Resent-From: Eli Zaretskii <eliz@gnu.org>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@gnu.org
Resent-Date: Sun, 19 Nov 2023 10:59:01 +0000
Resent-Message-ID: <handler.67275.B67275.170039151129627@debbugs.gnu.org>
Resent-Sender: help-debbugs@gnu.org
X-GNU-PR-Message: followup 67275
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
Original-Received: via spool by 67275-submit@debbugs.gnu.org id=B67275.170039151129627
 (code B ref 67275); Sun, 19 Nov 2023 10:59:01 +0000
Original-Received: (at 67275) by debbugs.gnu.org; 19 Nov 2023 10:58:31 +0000
Original-Received: from localhost ([127.0.0.1]:50100 helo=debbugs.gnu.org)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
 id 1r4fVX-0007hm-4U
 for submit@debbugs.gnu.org; Sun, 19 Nov 2023 05:58:31 -0500
Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34568)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@gnu.org>) id 1r4fVT-0007hW-Sn
 for 67275@debbugs.gnu.org; Sun, 19 Nov 2023 05:58:30 -0500
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 <eliz@gnu.org>)
 id 1r4fVM-000538-Sf; Sun, 19 Nov 2023 05:58:20 -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=G3MLiFKZ+LtKZSTpq1GJsc2NFt5lUro+wwv/k/pn/n0=; b=NxLy+RXKRH/U
 nzxp8B5RvQeEFvcCokloGN1XyMBlKoczfxl5EQX/oVQOeIlro/IvxAKiOnJShm7eJ5fJd0Gu2783P
 6vz31n4neIo9ba1mrRsbk/7mcOM0DJwS0mO/b9CEaSoeDLD0/dwNHHFQCPqbXwKKZ6Ql4nBJY7BmD
 l23P4WYPkbWsOObA5oOhMIZELSWCEpCCkSSiA52F2YtpVJdXGYt4irgjtsHPoxRzg9nxwQ/yvYIFR
 RVi0WmAk0OnnCMpv8h3XRlDDSNDtz2ocNVvkqsjHZmWUy6d7qLx3hciY3nQz6ypfPaaZH5nKsudQv
 2L4MOheRkAbR38+u0htcaA==;
In-Reply-To: <m1sf529gzw.fsf@dazzs-mbp.home> (bug-gnu-emacs@gnu.org)
X-BeenThere: debbugs-submit@debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
X-BeenThere: bug-gnu-emacs@gnu.org
List-Id: "Bug reports for GNU Emacs,
 the Swiss army knife of text editors" <bug-gnu-emacs.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/bug-gnu-emacs>
List-Post: <mailto:bug-gnu-emacs@gnu.org>
List-Help: <mailto:bug-gnu-emacs-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=subscribe>
Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org
Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org
Xref: news.gmane.io gmane.emacs.bugs:274607
Archived-At: <http://permalink.gmane.org/gmane.emacs.bugs/274607>

> Date: Sun, 19 Nov 2023 11:25:55 +0100
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> +(defun completion-preview--try-table (table beg end props)
> +  "Check TABLE for a completion matching the text between BEG and END.
> +
> +PROPS is a property list with additional information about TABLE.
> +See `completion-at-point-functions' for more details.
> +
> +When TABLE contains a matching completion, return a list
> +\(PREVIEW BEG END ALL EXIT-FN) where PREVIEW is the text to show
> +in the completion preview, ALL is the list of all matching
> +completion candidates, and EXIT-FN is either a function to call
> +after inserting PREVIEW or nil.  When TABLE does not contain
> +matching completions, or when there are multiple matching
> +completions and `completion-preview-exact-match-only' is non-nil,
> +return nil instead."

It is better to use "if" here where you use "when".  "When" can be
interpreted as a time-related condition, which is not what you want
here.

> +(defun completion-preview--capf-wrapper (capf)
> +  "Translate output of CAPF to properties for completion preview overlay.
> +
> +If CAPF returns a list (BEG END TABLE . PROPS), call

If CAPF _returns_ something, it is probably a function.  But then why
does the first sentence say "output of CAPF"? functions in ELisp don't
"output" stuff.

> +`completion-preview--try-table' to check TABLE for matching
> +completion candidates.  If `completion-preview--try-table'
> +returns a non-nil value, return that value.  Otherwise, return a
> +list with nil car which means that completion failed, unless
> +PROPS includes the property `:exclusive' with value `no', in
> +which case this function returns nil which means to try other
> +functions from `completion-at-point-functions'."

This basically tells in words what the code does.  But since the code
is quite simple, I wonder why we need this in the doc string.  The
disadvantage of having this in the doc string is that we'd need to
update it each time the code changes.

Instead, think if something in what the code does needs to be
explained _beyond_ what the code itself says, like if you need to
explain the reasons why the code does what it does, or why you access
this or that property, and explain that -- in comments, not in the doc
string.  The doc string should ideally be a higher-level description
of the function's purpose and the meaning of its return values.

Thanks.