From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Mauro Aranda Newsgroups: gmane.emacs.bugs Subject: bug#38812: 28.0.50; Custom: Problem with reverting some session's customizations Date: Tue, 31 Dec 2019 14:38:02 -0300 Message-ID: References: <83o8volbvf.fsf@gnu.org> <83eewkl858.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000bb870f059b036cf3" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="10386"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 38812@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Dec 31 18:39:11 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1imLU7-0002bH-Au for geb-bug-gnu-emacs@m.gmane.org; Tue, 31 Dec 2019 18:39:11 +0100 Original-Received: from localhost ([::1]:44998 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1imLU6-0006xW-1N for geb-bug-gnu-emacs@m.gmane.org; Tue, 31 Dec 2019 12:39:10 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46363) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1imLTz-0006v9-VH for bug-gnu-emacs@gnu.org; Tue, 31 Dec 2019 12:39:05 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1imLTy-00015D-JD for bug-gnu-emacs@gnu.org; Tue, 31 Dec 2019 12:39:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57324) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1imLTy-00014v-GG for bug-gnu-emacs@gnu.org; Tue, 31 Dec 2019 12:39:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1imLTy-000573-F0 for bug-gnu-emacs@gnu.org; Tue, 31 Dec 2019 12:39:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Mauro Aranda Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 31 Dec 2019 17:39:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 38812 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 38812-submit@debbugs.gnu.org id=B38812.157781390319605 (code B ref 38812); Tue, 31 Dec 2019 17:39:02 +0000 Original-Received: (at 38812) by debbugs.gnu.org; 31 Dec 2019 17:38:23 +0000 Original-Received: from localhost ([127.0.0.1]:35064 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1imLTK-000568-Ju for submit@debbugs.gnu.org; Tue, 31 Dec 2019 12:38:23 -0500 Original-Received: from mail-lj1-f179.google.com ([209.85.208.179]:34309) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1imLTJ-00055t-1V for 38812@debbugs.gnu.org; Tue, 31 Dec 2019 12:38:21 -0500 Original-Received: by mail-lj1-f179.google.com with SMTP id z22so32075126ljg.1 for <38812@debbugs.gnu.org>; Tue, 31 Dec 2019 09:38:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KvscVNo5NW/glOXvw5XS58xUQnKIMTrZr4VFnEEN+ic=; b=HZLzmF377SN4ThBzROsxs9Yx103YOnuTTlvGxtvova6EPRpay/qgK0ry7DCW1LtkQ/ U8NNatqj+HQRZHAmrNzXxzUelhOx35yvCCLg4l1ZOLmZUAcvPIf4r1dbYtxNidOn2YQS IzYaRym/tdPNjVw9ElBMCcpq8hEFb8gBUcjlcrcAWD9md3F/awloRUtvlrhi8smifFDd 5lB4ZiVzBet1ku0uPx29e1sKBPabrN8c4yfqf2N0z/ICyg4x7PSz+JoK/WiDXV6e6FwR Ipyzbw2eiVxGFkePqREKTRwLbi6owRgUEUHs9/MVPZmz3RD/0QrmfyJcjfglgFobMsX4 v/rQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KvscVNo5NW/glOXvw5XS58xUQnKIMTrZr4VFnEEN+ic=; b=XcF5OIOIgrRRo68NBYoOiyNmQ/L6Owf6llsgUMbLSUvF2vGkJWAzwX4MhQFwpQa42o SuhMhIIYQmuWqunpflmhGfe5JO7zwaIsPr582erTahEfK2MWk9rrgpDKui24Jqa2oquZ Zw7oQchSL2BT1gIGmqwILP73v7rfeGHvNm+LFkGb/xU/DRD5ZkdlGGmeSD+/hTmo9t9e UJX+MC08rpHwpkcER8TMy7nwZxF5NCJ/N73xQe9XwioiCyFZKUN70YU5N4CjH2wV0CNO JlYcwumtkQC7XZfARVrViY1bbNxxynbLS/gHyniQAVy+6kMiavQb5SofzRpeGPeob7xh WCew== X-Gm-Message-State: APjAAAX9ho7tOEDCBXnhSIRgvn4igEinG97wNLLF5g+ZKlltZxpaWWs6 uZSdId3PWsmGb6kqNMoJoRXcRS2HPPnSL+E4KzQ= X-Google-Smtp-Source: APXvYqx2xI1L36M34bx5OcT+KFVXgYgy2lPtVTaOVk4K7bE6NzVvKf7LLWakyQ4pdpZB3kVuUCpr5YuskNKTHBoUewM= X-Received: by 2002:a2e:96c6:: with SMTP id d6mr12954280ljj.4.1577813894988; Tue, 31 Dec 2019 09:38:14 -0800 (PST) In-Reply-To: <83eewkl858.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:173992 Archived-At: --000000000000bb870f059b036cf3 Content-Type: text/plain; charset="UTF-8" Eli Zaretskii writes: >> From: Mauro Aranda >> Date: Tue, 31 Dec 2019 14:21:09 -0300 >> Cc: 38812@debbugs.gnu.org >> >> > And I have a question: isn't it better not to use setcar in >> > custom-push-theme instead? >> >> I thought of doing that, and use setf with alist-get to make the change >> instead. But I think we'll be better off if we avoid sharing the cons >> cell inadvertedly, since that is prone to have bugs like this one. > > And I actually think that a problem should be fixed where it is > caused. There's nothing wrong per se with sharing portions of Lisp > data structures. Of course. But I said "inadvertedly". Now we are aware. >> Alternatively, we could create the list in custom-theme-recalc-variable, >> to accomplish the same thing without changing the return value of >> custom-variable-theme-value. In that case, I think it would be >> convenient to change the doc string of custom-variable-theme-value, to >> say it returns some cdr. >> >> To me, either the patch I posted (with an additional explanatory >> comment, of course) or the latter option sound better, but I won't argue >> too much if you think otherwise. > > My alternative patch is below. WDYT? > > diff --git a/lisp/custom.el b/lisp/custom.el > index 26bdaae2c2..7ed85b22e8 100644 > --- a/lisp/custom.el > +++ b/lisp/custom.el > @@ -886,7 +886,10 @@ custom-push-theme > (put theme 'theme-settings > (cons (list prop symbol theme value) > (delq res theme-settings))) > - (setcar (cdr setting) value))) > + ;; It's tempting to use setcar here, but that could > + ;; inadvertently modify other properties in SYMBOL's proplist, > + ;; if those just happen to share elements with the value of PROP. > + (put symbol prop (cons (list theme value) (delq setting old))))) > ;; Add a new setting: > (t > (when (custom--should-apply-setting theme) Looks good, thank you. --000000000000bb870f059b036cf3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Eli Zaretskii <eliz@gnu.org> writes:

>> From: Mauro Aranda &l= t;maurooaranda@gmail.com><= br>>> Date: Tue, 31 Dec 2019 14:21:09 -0300
>> Cc: 38812@debbugs.gnu.org
>> >> > And I have a question: isn't it better not to use setcar= in
>> > custom-push-theme instead?
>>
>> I = thought of doing that, and use setf with alist-get to make the change
&g= t;> instead.=C2=A0 But I think we'll be better off if we avoid shari= ng the cons
>> cell inadvertedly, since that is prone to have bugs= like this one.
>
> And I actually think that a problem should = be fixed where it is
> caused.=C2=A0 There's nothing wrong per se= with sharing portions of Lisp
> data structures.

Of course.= =C2=A0 But I said "inadvertedly".=C2=A0 Now we are aware.

= >> Alternatively, we could create the list in custom-theme-recalc-var= iable,
>> to accomplish the same thing without changing the return= value of
>> custom-variable-theme-value.=C2=A0 In that case, I th= ink it would be
>> convenient to change the doc string of custom-v= ariable-theme-value, to
>> say it returns some cdr.
>> >> To me, either the patch I posted (with an additional explanatory=
>> comment, of course) or the latter option sound better, but I w= on't argue
>> too much if you think otherwise.
>
>= My alternative patch is below.=C2=A0 WDYT?
>
> diff --git a/li= sp/custom.el b/lisp/custom.el
> index 26bdaae2c2..7ed85b22e8 100644> --- a/lisp/custom.el
> +++ b/lisp/custom.el
> @@ -886,7 = +886,10 @@ custom-push-theme
> =C2=A0 (put theme 'theme-settings<= br>> =C2=A0 =C2=A0 =C2=A0 (cons (list prop symbol theme value)
> = =C2=A0 =C2=A0 (delq res theme-settings)))
> - (setcar (cdr setting)= value)))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0;; It's tempting to use = setcar here, but that could
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0;; inadver= tently modify other properties in SYMBOL's proplist,
> + =C2=A0 = =C2=A0 =C2=A0 =C2=A0;; if those just happen to share elements with the valu= e of PROP.
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0(put symbol prop (cons (lis= t theme value) (delq setting old)))))
> =C2=A0 =C2=A0 =C2=A0 ;; Add a= new setting:
> =C2=A0 =C2=A0 =C2=A0 (t
> =C2=A0 =C2=A0 =C2=A0 = =C2=A0(when (custom--should-apply-setting theme)

Looks good, thank y= ou.
--000000000000bb870f059b036cf3--