bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#62847: 29.0.90; Propertized space in Org Agenda's mode-name


From: Stefan Monnier
Subject: bug#62847: 29.0.90; Propertized space in Org Agenda's mode-name
Date: Sat, 15 Apr 2023 09:21:35 -0400
User-agent: Gnus/5.13 (Gnus v5.13)

> So, it really looks like compilation problem.
>
> I am now looking into Elisp manual
>
>     2.9 Mutability
>     
>        When similar constants occur as parts of a program, the Lisp
>     interpreter might save time or space by reusing existing constants or
>     their components.  For example, ‘(eq "abc" "abc")’ returns ‘t’ if the
>     interpreter creates only one instance of the string literal ‘"abc"’, and
>     returns ‘nil’ if it creates two instances.  Lisp programs should be
>     written so that they work regardless of whether this optimization is in
>     use.
>
> So, it should be a good idea to avoid setting text properties in string
> constants in general.

Indeed, since it modifies the object, it's "undefined behavior" territory.

> -       (propertize "ALL" 'face 'org-agenda-structure-filter)
> +       (propertize
> +        (copy-sequence "ALL") ; Avoid modifying `eq' string constants.
> +        'face 'org-agenda-structure-filter)

`propertize` does not modify its string argument (it returns a new
string instead), so the `copy-sequence` here is a pure waste.

> -                             (propertize " " 'display category-icon)
> +                             (propertize
> +                                 (copy-sequence " ") ; Avoid modifying `eq' 
> " ".
> +                                 'display category-icon)

Same here.

> -                     (org-add-props " " (org-plist-delete 
> (text-properties-at 0 x)
> -                                                          'display)))
> +                     (org-add-props
> +                         (copy-sequence " ") ; Avoid modifying `eq' " ".
> +                         (org-plist-delete (text-properties-at 0 x)
> +                                           'display)))

This hunk fixes a real bug, OTOH.
Maybe you should use `(apply #'propertize` instead or `org-add-props`?


        Stefan






reply via email to

[Prev in Thread] Current Thread [Next in Thread]