From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Newsgroups: gmane.emacs.bugs Subject: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting Date: Sat, 14 Aug 2021 11:36:32 +0100 Message-ID: <87y29471ov.fsf@gmail.com> References: <3d3f894f-a6fa-53ae-5d50-c3aa9bffc73e@daniel-mendler.de> <56ab18b1-4348-9b2c-85bb-af9b76cd429a@daniel-mendler.de> <38a06f3c-4a7a-755c-c99b-708f91afabfa@daniel-mendler.de> <9f59f87c-2489-aaa0-5b3f-0e911b7ffa6c@daniel-mendler.de> <8a36e61a-1c5b-bf3b-a454-077348589c4f@yandex.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31193"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Daniel Mendler , Stefan Monnier , 48841@debbugs.gnu.org, 47711@debbugs.gnu.org To: Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Aug 14 12:37:15 2021 Return-path: 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 ) id 1mEr2Q-0007uy-7Z for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 14 Aug 2021 12:37:14 +0200 Original-Received: from localhost ([::1]:47526 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mEr2O-0003IQ-T9 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 14 Aug 2021 06:37:12 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:43820) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mEr2E-0003I8-HF for bug-gnu-emacs@gnu.org; Sat, 14 Aug 2021 06:37:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:60321) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mEr2E-0002y4-Ab for bug-gnu-emacs@gnu.org; Sat, 14 Aug 2021 06:37:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mEr2E-0002j4-8E for bug-gnu-emacs@gnu.org; Sat, 14 Aug 2021 06:37:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 14 Aug 2021 10:37:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 48841 X-GNU-PR-Package: emacs Original-Received: via spool by 48841-submit@debbugs.gnu.org id=B48841.162893740910447 (code B ref 48841); Sat, 14 Aug 2021 10:37:02 +0000 Original-Received: (at 48841) by debbugs.gnu.org; 14 Aug 2021 10:36:49 +0000 Original-Received: from localhost ([127.0.0.1]:43631 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEr1x-0002iM-II for submit@debbugs.gnu.org; Sat, 14 Aug 2021 06:36:49 -0400 Original-Received: from mail-wr1-f54.google.com ([209.85.221.54]:37387) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEr1s-0002i1-Db; Sat, 14 Aug 2021 06:36:44 -0400 Original-Received: by mail-wr1-f54.google.com with SMTP id r6so16717117wrt.4; Sat, 14 Aug 2021 03:36:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=Bh27k0LEsx5DFXwN7gWYs7DxXUNYaLIjbu6ommRbxA8=; b=kY3CHHhQPgxYfZeYOt4QG8fG3N2dFqnffJS3kMYkIjvl+wEvD+BS/sHE2NuChbqzAa D4HElyd6MqwzNTwVJaeT6FQ6LKwoqIKm4g0yKJ3oQpeBsg4se6oPjNTzKkumbkDBYGpr pjpnT5CF3bNOU14bbK7Z2PbQs0inEKQOge+OABAkZsSrm69W2Hp4yc5MSP9N6BQzM9Ay LK/z2biTj28/xjdOe/Q7EGHgiTIdxCks/Rm5TSYR+fIYJAtdABmAaXaKpNb0Qh/aDkrt YhWQHObfly+w0+D5TeJFYHrObATriUCqu6NsJcCTmE+SqBx+ywu14VGjwCJE0W4iVReh 6oKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=Bh27k0LEsx5DFXwN7gWYs7DxXUNYaLIjbu6ommRbxA8=; b=GFi1kooyF9K6kD5WOhruUZCsgahBUr/OtCP0+7FSU4jMRgCUT6On70fIkPBpX0QiVd DSQF9O01zrwPhfVtlaTtoNwZIdsEYL8sSU48VjrsGEuylvv0ScV16TI78DjjF6Gl4MPW 7uc0ZaMetXeX0R9jmqJZS/KK1VfAE6oeFERbBdiSB12UiNLyK6uPZORmgaZI0h3okVnV ExRZsX7kjA47zhr9wEv9fN2+wJiXGarW4fHTn6qL74nzAB44Cfi85e89eUF1La2eEqjq Y5VoXqiAZfNBlt1o+FIzmxdM7uAGXnUBVrUqOXYHwfeGAsfJrj4SXO9x9g7JgotTECeH UlHQ== X-Gm-Message-State: AOAM531Hp91nzm9zqA0llTP2Yx2Ujt2//uZ1QKC4J1UNmCeNa/NwU7Lk bhax5eEIMhKH6PrY6j4mit5GwNMgdfM= X-Google-Smtp-Source: ABdhPJzhS09Z/f61UZIaCPeue+OeZ21p0Mzyvuu4983LqVr6GkAUSlSIt2ZLLV39lmhe7drkDbsGBg== X-Received: by 2002:adf:e107:: with SMTP id t7mr7755483wrz.165.1628937394463; Sat, 14 Aug 2021 03:36:34 -0700 (PDT) Original-Received: from krug (a94-133-27-132.cpe.netcabo.pt. [94.133.27.132]) by smtp.gmail.com with ESMTPSA id z137sm4350807wmc.14.2021.08.14.03.36.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 14 Aug 2021 03:36:33 -0700 (PDT) In-Reply-To: <8a36e61a-1c5b-bf3b-a454-077348589c4f@yandex.ru> (Dmitry Gutov's message of "Sat, 14 Aug 2021 05:47:43 +0300") 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" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:211818 Archived-At: Dmitry Gutov writes: > It's basically this: we cannot mutate what we don't own. Across all of > completion functions out there, there will be such that return > "shared" strings (meaning, not copied or newly allocated) from their > completion tables. And modifying them is bad, with consequences which > can present themselves in unexpected, often subtle ways. I agree with this premise. But would you call putting a uniquely named text property in them a modification or mutation of said strings? I don't. > Since up until now completion-pcm--hilit-commonality copied all > strings before modifying, completion tables such as described (with > "shared" strings) have all been "legal". Again, if I take one of this shared strings, in whichever environment running row now, and I secretly attach a privatte "joaot/blargh" property to it, there is very very low likelyhood that that will hurt anybody. You seem to be worrying about re-setting the 'face' property (a public property by excellence) and that's the very same bug I experienced and have described early. It's not even a hard bug to see. Just remove the copy-sequence in `completion-pcm--hilit-commonality' and see strange stuff happening. But if you set some other property, _that_ bug _doesn't_ occur. Do some other bugs occur? I don't know, I don't think we'll ever know, for _any_ change. Furthermore, there are other ways to forego the copying in `completion-pcm--hilit-commonality and not even touch _ANY_ string property. > Suddenly deciding to stop supporting them would be a major API > breakage with consequences that are hard to predict. And while I > perhaps agree that it's an inconvenience, I don't think it's a choice > we can simply make as this stage in c-a-p-f's development. > > To give an example of a subtle consequence: > > 1. (setq s (symbol-name 'car)) > > 2. (put-text-property 1 3 'face 'error s) > > 3. Switch to a buffer in fundamental mode > > 4. (insert (symbol-name 'car)) --> see the error face in the buffer It's not even subtle :-) Yes this is why have seen from the beginning in bug#48841. I think it was even I who reported it to you. The principle to follow can be summarized as this: "Don't touch values of properties you don't own in objects you don't own." So just don't touch the 'face' property in things you don't own! But feel free to touch the "dmitry/blargh" property even in objects you don't own. So 'c-p--h-l' doesn't "own" face. So it must either create an object that it owns or set something that it does own. 'completion-score' is "owned" by 'c-p--h-l'. Only it can write it (though others can read it). > Now imagine that some completion table collects symbol names by > passing obarray through #'symbol-name rather than #'all-completions, > and voila, if the completion machinery adds properties (any > properties, not just face) to those strings, you have just modified a > bunch of global values. That's not good. Why? Maybe I'm missing something. Why is adding properties -- that no-one but the completion machinery knows about -- to those shared strings "not good"? What bad thing can happen if I do? > And in the example above, the values are those that the > lispref/objects.texi says we should not change (though it gives > (symbol-name 'cons) as example). "Not mutable", in its parlance. IIRC > the related discussions mentioned that modifying such values could > lead to a segfault in some previous Emacs versions. Maybe not anymore, > but it's still not a good idea. You're extrapolating "change" to also include attaching properties to symbols. I think that document just means that you can't do stuff like (aset "cons" 0 ?z) or (aset (symbol-name 'cons) 0 ?z) I don't think it means you can't (put-text-property 0 1 'joaot/muahahah 42 (symbol-name 'cons)) But maybe Eli or someone else more knowledgeable in the deep internals of Emacs can correct me. If indeed I'm wrong, there are other ways to forego the copying in `c-p---hilit-commonality` and still don't incurr in any such "mutation". We must keep our eyes on the prize: copying -- not property-attaching -- is the real bummer here. scratch/icomplete-lazy-highlight-attempt-2, although still incomplete, is one such approach, though it still sets `completion-score` on the "shared" string, used later for sorting. But also that could be prevented (again, only if it turns out to be actually problematic IMO). Jo=C3=A3o PS: Maybe I've not stated it clearly enough: I *don't* object to -- or endorse -- Daniel's patch. My point was solely that it mixes too many things for me to be intellectually able to review its functional merits, and that those things should be separated into multiple problems and patches to make this evaluation easier. Maybe someone with superior intellecutal capacity can review -- on substance -- as it stands. See my other reply containing benchmarks. Daniel's patch doesn't perform well there, but for all I know, it can co-exist with my non-copying approach, and we can all have our cake.