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#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 10:42:16 +0200
Message-ID: <83edgedy1j.fsf@gnu.org>
References: <ier8r6s42on.fsf@janestreet.com>
 <dc5ad8f4-b857-ea87-d178-bae903cee517@gutov.dev>
 <421e3ea8-d150-566e-f645-cbf1ccbcba61@gutov.dev>
 <ier34wz3zkx.fsf@janestreet.com>
 <81b6ed58-3fdc-3b5a-41a9-84d66475d5f1@gutov.dev>
 <ierr0khlq19.fsf@janestreet.com>
 <5c3eb6a1-38da-8af4-419a-e0567b163e3a@gutov.dev>
 <iercyw1l6rz.fsf@janestreet.com> <83bkblgeja.fsf@gnu.org>
 <ab8f5ad1-356e-e90d-eab4-d36a0b0821fc@gutov.dev>
Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214";
	logging-data="21662"; mail-complaints-to="usenet@ciao.gmane.io"
Cc: 67310@debbugs.gnu.org, sbaugh@janestreet.com, juri@linkov.net
To: Dmitry Gutov <dmitry@gutov.dev>
Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Nov 25 09:43:23 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 1r6oG2-0005RU-Hr
	for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 25 Nov 2023 09:43:22 +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 1r6oFf-0004M1-3o; Sat, 25 Nov 2023 03:42:59 -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 1r6oFd-0004LZ-7X
 for bug-gnu-emacs@gnu.org; Sat, 25 Nov 2023 03:42:57 -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 1r6oFc-0007v3-W3
 for bug-gnu-emacs@gnu.org; Sat, 25 Nov 2023 03:42:57 -0500
Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2)
 (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1r6oFh-0008EL-NN
 for bug-gnu-emacs@gnu.org; Sat, 25 Nov 2023 03:43:01 -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: Sat, 25 Nov 2023 08:43:01 +0000
Resent-Message-ID: <handler.67310.B67310.170090175431602@debbugs.gnu.org>
Resent-Sender: help-debbugs@gnu.org
X-GNU-PR-Message: followup 67310
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: patch
Original-Received: via spool by 67310-submit@debbugs.gnu.org id=B67310.170090175431602
 (code B ref 67310); Sat, 25 Nov 2023 08:43:01 +0000
Original-Received: (at 67310) by debbugs.gnu.org; 25 Nov 2023 08:42:34 +0000
Original-Received: from localhost ([127.0.0.1]:37748 helo=debbugs.gnu.org)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
 id 1r6oFG-0008De-1s
 for submit@debbugs.gnu.org; Sat, 25 Nov 2023 03:42:34 -0500
Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49672)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@gnu.org>) id 1r6oFD-0008DP-BE
 for 67310@debbugs.gnu.org; Sat, 25 Nov 2023 03:42:32 -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 1r6oF2-0007oB-10; Sat, 25 Nov 2023 03:42: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=LuZUYz2JUXU6jRMxH2ZUJwVXGA1OEJfvRIoUaNdyIfU=; b=lIpW5Ozz64lO
 v0SlM4f6YLA/g/WNbQvV1Rh2GuQsdUlDjZJDMjZBihZnJ1z/dknTsHizh0QI89Zh0269ODf5xtVy9
 V9ouu2GIhhhOOI4LIV5kaU+0temlVXm4/dMLCg2Ly31n8FO69eXpJhlUt1UM6RFoluMIr6BeEvkmU
 2YrKyjOTANYaVy/wKEGqmEnlyN5Ag80fpEfWhxNmywoJZlwPjwKTGoVGU/6Uie3sq2pbMeEJuA2cn
 58gsCqG4ZIpYQ6083YT4LVZAwTKjvuRC2In2GlczIvmgXapcUd90ZoTXeAaWu7EGtHaoKST/J7R6R
 PvEDGpVkTL1as/kXYHbkmw==;
In-Reply-To: <ab8f5ad1-356e-e90d-eab4-d36a0b0821fc@gutov.dev> (message from
 Dmitry Gutov on Sat, 25 Nov 2023 03:54:13 +0200)
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:274925
Archived-At: <http://permalink.gmane.org/gmane.emacs.bugs/274925>

> Date: Sat, 25 Nov 2023 03:54:13 +0200
> Cc: 67310@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> I'll try to answer some of the questions that are still relevant to the 
> latest patch, myself.
> 
> On 23/11/2023 08:38, Eli Zaretskii wrote:
> 
> >> +            ;; Iterate in reverse order so project--name-history is in
> >> +            ;; the correct order.
> > 
> > What is the "correct" order?
> 
> Their order in project--list.
> 
> Iteration and construction of a new list with 'push' leads to the 
> reverse order, hence the use of reversion at the beginning to counteract 
> that.

Then the comment should say

  Iterate in reverse order so project--name-history is in the same
  order as project--list.

> >> -              (if-let (proj (project--find-in-directory dir))
> >> -                  (push (cons (project-name proj) proj) ret)))
> >> +              (when-let (proj (project--find-in-directory dir))
> >> +                (let ((name (project-name proj)))
> >> +                  (push name project--name-history)
> >> +                  (push (cons name proj) ret))))
> > 
> > Not sure I understand why you replaced if-let with when-let here.
> 
> To reduce the amount of indentation, perhaps.

Why is that an advantage?

I generally request and expect people not to make unnecessary changes,
since doing that makes forensics harder: you see changes which don't
change the code's semantics, and need to waste time studying such
"changes" and deciding that they are no-ops.

Please everyone keep this in mind when you make changes.

> >> +        (let ((history-delete-duplicates t)
> >> +              (history-length t))
> >> +          (add-to-history 'project--list root))
> > 
> > Why are you overriding the values of these two user options?
> 
> To implement the current behavior (how additions to project--list) 
> happen. I've described that behavior in one of the earlier messages here.

I think this is not a good idea, regardless of the reasons.  Users
have these options to control how history functionality behaves in
Emacs, and here you take away that control with no "fire escape".

As for the description you allude to above, all I found is this, which
is part of Spencer's commit log:

  The project--list is already ordered such that the most recently used
  projects are at the front.  Now we use it as the minibuffer history
  when prompting for a project.

  To avoid savehist from picking up project--list as a minibuffer
  history variable and overriding our own persistence mechanism, we
  don't pass project--list directly as a history variable, but instead
  pass project--dir-history or project--name-history, dynamically-bound
  to an appropriate value.  project--dir-history and
  project--name-history won't be persisted since they're always unbound
  at the top level; but if they are persisted anyway somehow, it won't
  affect us.

  If we later find a way to rely on savehist for persistence instead of
  having our own mechanism, we can change the in-memory format of
  project--list to be just a list of directories, and our explicit calls
  to project--add-dir can be replaced by let-binding
  history-delete-duplicates=t, history-length=t.

If this is what you mean, then I don't see how this justifies the
overriding.