bug#30786: Save text properties in desktop

bug#30786: Save text properties in desktop

[Juri Linkov]

Why does desktop.el not save text properties?
Maybe for some historical reasons?
This is required for bug#22479.
I can't find a problem while trying this patch:

diff --git a/lisp/desktop.el b/lisp/desktop.el
index 8bd4465..a7e549f 100644
--- a/lisp/desktop.el
+++ b/lisp/desktop.el
@@ -852,10 +852,7 @@ desktop--v2s
     ((or (numberp value) (null value) (eq t value) (keywordp value))
      (cons 'may value))
     ((stringp value)
-     (let ((copy (copy-sequence value)))
-       (set-text-properties 0 (length copy) nil copy)
-       ;; Get rid of text properties because we cannot read them.
-       (cons 'may copy)))
+     (cons 'may value))
     ((symbolp value)
      (cons 'must value))
     ((vectorp value)

bug#30786: Save text properties in desktop

[Noam Postavsky]

Juri Linkov <juri@linkov.net> writes:

> Why does desktop.el not save text properties?
> Maybe for some historical reasons?
> This is required for bug#22479.
> I can't find a problem while trying this patch:

> -     (let ((copy (copy-sequence value)))
> -       (set-text-properties 0 (length copy) nil copy)
> -       ;; Get rid of text properties because we cannot read them.

I guess this comment is relevant, you can easily have non-readable
property values, e.g.: (put-text-property 1 2 'foo (point-marker)).
Maybe solving Bug#24982 would help?

bug#30786: Save text properties in desktop

[Juri Linkov]

>> Why does desktop.el not save text properties?
>> Maybe for some historical reasons?
>> This is required for bug#22479.
>> I can't find a problem while trying this patch:
>
>> -     (let ((copy (copy-sequence value)))
>> -       (set-text-properties 0 (length copy) nil copy)
>> -       ;; Get rid of text properties because we cannot read them.
>
> I guess this comment is relevant, you can easily have non-readable
> property values, e.g.: (put-text-property 1 2 'foo (point-marker)).

If the only problem is with non-readable property values then we could
check for such values and not to write them to the desktop file.

> Maybe solving Bug#24982 would help?

This would help on reading the desktop, but maybe better not to save
non-readable values in the first place.

bug#30786: Save text properties in desktop

[Drew Adams]

> > I guess this comment is relevant, you can easily have non-readable
> > property values, e.g.: (put-text-property 1 2 'foo (point-marker)).
> 
> If the only problem is with non-readable property values then we could
> check for such values and not to write them to the desktop file.
> 
> > Maybe solving Bug#24982 would help?
> 
> This would help on reading the desktop, but maybe better not to save
> non-readable values in the first place.

No.  If the problem is reading then that's where the solution
should be located - not writing.  It has happened quite a few
times that something unreadable by Emacs has later become
readable.

One of the important motivations behind bug #24982 is to let
Emacs version N be able to read (by ignoring) constructs that
it finds are unreadable but that are readable in Emacs version
N + M.

The sooner we get that bug fixed, the more versions of Emacs
will be able to benefit from it.  Users and code should be
able to make Emacs tolerant of read errors (by ignoring them).

We have `condition-case' for most errors.  We have little or
no control over read errors.

Dunno whether we need a complete read-error-handling construct
a la `condition-case', but we at least need a way to let Emacs
ignore all read errors.  And preferably within some scope (e.g.
let-bind a variable).

bug#30786: Save text properties in desktop

[Noam Postavsky]

Drew Adams <drew.adams@oracle.com> writes:

>> If the only problem is with non-readable property values then we could
>> check for such values and not to write them to the desktop file.
>> 
>> > Maybe solving Bug#24982 would help?
>> 
>> This would help on reading the desktop, but maybe better not to save
>> non-readable values in the first place.
>
> No.  If the problem is reading then that's where the solution
> should be located - not writing.  It has happened quite a few
> times that something unreadable by Emacs has later become
> readable.

You mean the print syntax changes to become readable?  But not that
Emacs can later read some unreadable #<...> syntax, right?

> We have `condition-case' for most errors.  We have little or
> no control over read errors.
>
> Dunno whether we need a complete read-error-handling construct
> a la `condition-case', but we at least need a way to let Emacs
> ignore all read errors.  And preferably within some scope (e.g.
> let-bind a variable).

`condition-case' works for read errors too, afaik, but you're suggesting
a different kind of control is needed.  Something more like Common Lisp
restart-case, I guess?

That could be useful in general, but solving this particular bug by
avoiding writing unreadable objects as Juri suggests seems okay too (and
much less work, hence more likely to actually happen instead of just
sitting for years).

bug#30786: Save text properties in desktop

[Drew Adams]

> >> > Maybe solving Bug#24982 would help?
> >>
> >> This would help on reading the desktop, but maybe better not to save
> >> non-readable values in the first place.
> >
> > No.  If the problem is reading then that's where the solution
> > should be located - not writing.  It has happened quite a few
> > times that something unreadable by Emacs has later become
> > readable.
> 
> You mean the print syntax changes to become readable?  But not that
> Emacs can later read some unreadable #<...> syntax, right?

I mean what I said further down: a given syntax is not readable
in Emacs version N (e.g. 20), and so raises an error there, but
it is readable in Emacs N+M (e.g. 22).  If we had provided a
way in version N of ignoring such error raising then that would
let you read the rest of a file (for example), ignoring syntax
that is illegal in Emacs N.

> > We have `condition-case' for most errors.  We have little or
> > no control over read errors.
> >
> > Dunno whether we need a complete read-error-handling construct
> > a la `condition-case', but we at least need a way to let Emacs
> > ignore all read errors.  And preferably within some scope (e.g.
> > let-bind a variable).
> 
> `condition-case' works for read errors too, afaik,

Can you specify a particular read error to handle/ignore?

> but you're suggesting a different kind of control is needed.
> Something more like Common Lisp restart-case, I guess?

Dunno.  I actually am not familiar with CL `restart-case', or
else I've forgotten it.  Perhaps it was added after CLTL1?

> That could be useful in general, but solving this particular bug by
> avoiding writing unreadable objects as Juri suggests seems okay too (and
> much less work, hence more likely to actually happen instead of just
> sitting for years).

It's fine not to write unreadable objects here.

It's better to provide a simple way to not provoke a read
error for such objects.  It's a read problem, not a write
problem.

bug#30786: Save text properties in desktop

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

>>> If the only problem is with non-readable property values then we could
>>> check for such values and not to write them to the desktop file.
>>>
>>> > Maybe solving Bug#24982 would help?

There is also bug#17090.

>>> This would help on reading the desktop, but maybe better not to save
>>> non-readable values in the first place.
>>
>> No.  If the problem is reading then that's where the solution
>> should be located - not writing.  It has happened quite a few
>> times that something unreadable by Emacs has later become
>> readable.
>
> You mean the print syntax changes to become readable?  But not that
> Emacs can later read some unreadable #<...> syntax, right?

Even when the print syntax becomes readable in later versions, we still
can't write such syntax because earlier Emacs versions should be able
to read the same desktop file.

> That could be useful in general, but solving this particular bug by
> avoiding writing unreadable objects as Juri suggests seems okay too (and
> much less work, hence more likely to actually happen instead of just
> sitting for years).

Do you think this patch covers all possible unreadable cases on writing?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: desktop-text-properties.patch --]
[-- Type: text/x-diff, Size: 794 bytes --]

diff --git a/lisp/desktop.el b/lisp/desktop.el
index 55ec71c..4f98658 100644
--- a/lisp/desktop.el
+++ b/lisp/desktop.el
@@ -841,10 +841,12 @@ desktop--v2s
     ((or (numberp value) (null value) (eq t value) (keywordp value))
      (cons 'may value))
     ((stringp value)
-     (let ((copy (copy-sequence value)))
-       (set-text-properties 0 (length copy) nil copy)
-       ;; Get rid of text properties because we cannot read them.
-       (cons 'may copy)))
+     ;; Get rid of unreadable text properties.
+     (if (ignore-errors (read (format "%S" value)))
+         (cons 'may value)
+       (let ((copy (copy-sequence value)))
+         (set-text-properties 0 (length copy) nil copy)
+         (cons 'may copy))))
     ((symbolp value)
      (cons 'must value))
     ((vectorp value)

bug#30786: Save text properties in desktop

[Noam Postavsky]

Juri Linkov <juri@linkov.net> writes:

> Even when the print syntax becomes readable in later versions, we still
> can't write such syntax because earlier Emacs versions should be able
> to read the same desktop file.

> Do you think this patch covers all possible unreadable cases on writing?

> +     ;; Get rid of unreadable text properties.
> +     (if (ignore-errors (read (format "%S" value)))
> +         (cons 'may value)
> +       (let ((copy (copy-sequence value)))
> +         (set-text-properties 0 (length copy) nil copy)
> +         (cons 'may copy))))

I think it won't cover the case where an object's print syntax is only
readable by the current Emacs version, and not an earlier one.  To
handle that you'll need to call desktop--v2s recursively, like in the
vectorp and consp branches.

bug#30786: Save text properties in desktop

[Juri Linkov]

>> Do you think this patch covers all possible unreadable cases on writing?
>>
>> +     ;; Get rid of unreadable text properties.
>> +     (if (ignore-errors (read (format "%S" value)))
>> +         (cons 'may value)
>> +       (let ((copy (copy-sequence value)))
>> +         (set-text-properties 0 (length copy) nil copy)
>> +         (cons 'may copy))))
>
> I think it won't cover the case where an object's print syntax is only
> readable by the current Emacs version, and not an earlier one.  To
> handle that you'll need to call desktop--v2s recursively, like in the
> vectorp and consp branches.

I don't understand how calling desktop--v2s recursively will ensure its
readability in earlier versions?  For example, if Emacs 27 will write to
the desktop file ‘#("foo" 0 3 (bar 42))’ how Emacs 19 can read it?

bug#30786: Save text properties in desktop

[Noam Postavsky]

Juri Linkov <juri@linkov.net> writes:

>>> Do you think this patch covers all possible unreadable cases on writing?
>>>
>>> +     ;; Get rid of unreadable text properties.
>>> +     (if (ignore-errors (read (format "%S" value)))
>>> +         (cons 'may value)
>>> +       (let ((copy (copy-sequence value)))
>>> +         (set-text-properties 0 (length copy) nil copy)
>>> +         (cons 'may copy))))
>>
>> I think it won't cover the case where an object's print syntax is only
>> readable by the current Emacs version, and not an earlier one.  To
>> handle that you'll need to call desktop--v2s recursively, like in the
>> vectorp and consp branches.
>
> I don't understand how calling desktop--v2s recursively will ensure its
> readability in earlier versions?  For example, if Emacs 27 will write to
> the desktop file ‘#("foo" 0 3 (bar 42))’ how Emacs 19 can read it?

Ah, I wasn't thinking of the #(...) specifically, rather about the
values of text properties, e.g.,

    (cl-defstruct foo f1 f2)
    (desktop--v2s (make-foo :f1 1 :f2 2)) ;=> (may . "Unprintable entity")

So desktop--v2s considers structs as unprintable, unlike read, hence
using desktop--v2s recursively instead of read will catch properties
that would otherwise be unreadable in Emacs 25.

If we want to support Emacs versions that don't even know about #(...)
then we can't write string properties at all, I guess.

bug#30786: Save text properties in desktop

[Juri Linkov]

>>>> Do you think this patch covers all possible unreadable cases on writing?
>>>>
>>>> +     ;; Get rid of unreadable text properties.
>>>> +     (if (ignore-errors (read (format "%S" value)))
>>>> +         (cons 'may value)
>>>> +       (let ((copy (copy-sequence value)))
>>>> +         (set-text-properties 0 (length copy) nil copy)
>>>> +         (cons 'may copy))))
>>>
>>> I think it won't cover the case where an object's print syntax is only
>>> readable by the current Emacs version, and not an earlier one.  To
>>> handle that you'll need to call desktop--v2s recursively, like in the
>>> vectorp and consp branches.
>>
>> I don't understand how calling desktop--v2s recursively will ensure its
>> readability in earlier versions?  For example, if Emacs 27 will write to
>> the desktop file ‘#("foo" 0 3 (bar 42))’ how Emacs 19 can read it?
>
> Ah, I wasn't thinking of the #(...) specifically, rather about the
> values of text properties, e.g.,
>
>     (cl-defstruct foo f1 f2)
>     (desktop--v2s (make-foo :f1 1 :f2 2)) ;=> (may . "Unprintable entity")
>
> So desktop--v2s considers structs as unprintable, unlike read, hence
> using desktop--v2s recursively instead of read will catch properties
> that would otherwise be unreadable in Emacs 25.
>
> If we want to support Emacs versions that don't even know about #(...)
> then we can't write string properties at all, I guess.

If we can afford continuing incrementing the desktop file version in
‘desktop-file-version’ doing this every time when support for reading more
syntaxes is added, thus preventing incompatibilities, then the patch above
would be the most reliable solution.

bug#30786: Save text properties in desktop

[Noam Postavsky]

Juri Linkov <juri@linkov.net> writes:

>>>>> +     ;; Get rid of unreadable text properties.
>>>>> +     (if (ignore-errors (read (format "%S" value)))
>>>>> +         (cons 'may value)
>>>>> +       (let ((copy (copy-sequence value)))
>>>>> +         (set-text-properties 0 (length copy) nil copy)
>>>>> +         (cons 'may copy))))

> If we can afford continuing incrementing the desktop file version in
> ‘desktop-file-version’ doing this every time when support for reading more
> syntaxes is added, thus preventing incompatibilities, then the patch above
> would be the most reliable solution.

You would also have to strip text properties according to the value of
desktop-file-version, right?

bug#30786: Save text properties in desktop

[Juri Linkov]

>>>>>> +     ;; Get rid of unreadable text properties.
>>>>>> +     (if (ignore-errors (read (format "%S" value)))
>>>>>> +         (cons 'may value)
>>>>>> +       (let ((copy (copy-sequence value)))
>>>>>> +         (set-text-properties 0 (length copy) nil copy)
>>>>>> +         (cons 'may copy))))
>
>> If we can afford continuing incrementing the desktop file version in
>> ‘desktop-file-version’ doing this every time when support for reading more
>> syntaxes is added, thus preventing incompatibilities, then the patch above
>> would be the most reliable solution.
>
> You would also have to strip text properties according to the value of
> desktop-file-version, right?

This won't help for the counterexample that you demonstrated earlier with
(put-text-property 1 2 'foo (point-marker)).  Whereas this patch does.

bug#30786: Save text properties in desktop

[Noam Postavsky]

Juri Linkov <juri@linkov.net> writes:

>>>>>>> +     ;; Get rid of unreadable text properties.
>>>>>>> +     (if (ignore-errors (read (format "%S" value)))
>>>>>>> +         (cons 'may value)
>>>>>>> +       (let ((copy (copy-sequence value)))
>>>>>>> +         (set-text-properties 0 (length copy) nil copy)
>>>>>>> +         (cons 'may copy))))
>>
>>> If we can afford continuing incrementing the desktop file version in
>>> ‘desktop-file-version’ doing this every time when support for reading more
>>> syntaxes is added, thus preventing incompatibilities, then the patch above
>>> would be the most reliable solution.
>>
>> You would also have to strip text properties according to the value of
>> desktop-file-version, right?
>
> This won't help for the counterexample that you demonstrated earlier with
> (put-text-property 1 2 'foo (point-marker)).  Whereas this patch does.

Sorry, I don't think I explained my point very well above.

Using `read' to check readability can only test which objects are
readable in the current Emacs version.  So if we rely on that, then we
can only reliably produce desktop files compatible with the current
version.  Which means the only thing that desktop-file-version helps
with is giving a slightly better error message, yes?

(I don't think this is necessarily a deal-breaker, as long as it's well
documented)

bug#30786: Save text properties in desktop

[Juri Linkov]

>>>>>>>> +     ;; Get rid of unreadable text properties.
>>>>>>>> +     (if (ignore-errors (read (format "%S" value)))
>>>>>>>> +         (cons 'may value)
>>>>>>>> +       (let ((copy (copy-sequence value)))
>>>>>>>> +         (set-text-properties 0 (length copy) nil copy)
>>>>>>>> +         (cons 'may copy))))
>>>
>>>> If we can afford continuing incrementing the desktop file version in
>>>> ‘desktop-file-version’ doing this every time when support for reading more
>>>> syntaxes is added, thus preventing incompatibilities, then the patch above
>>>> would be the most reliable solution.
>>>
>>> You would also have to strip text properties according to the value of
>>> desktop-file-version, right?
>>
>> This won't help for the counterexample that you demonstrated earlier with
>> (put-text-property 1 2 'foo (point-marker)).  Whereas this patch does.
>
> Sorry, I don't think I explained my point very well above.
>
> Using `read' to check readability can only test which objects are
> readable in the current Emacs version.  So if we rely on that, then we
> can only reliably produce desktop files compatible with the current
> version.  Which means the only thing that desktop-file-version helps
> with is giving a slightly better error message, yes?
>
> (I don't think this is necessarily a deal-breaker, as long as it's well
> documented)

Yes, just better error handling (like what in bug#24982 was asked to do),
nothing more, so it's not a deal-breaker.

Currently I see no better way than in the aforementioned patch.
At least it provides 100% guarantee of readability of the desktop file
in the same version that wrote it.

But earlier versions will have an incompatibility problem anyway.
Let's suppose that Emacs 27 will support reading point-markers and writes
them to the desktop file for good.  Then naturally Emacs 26 will fail
to read such desktop files either way: explicitly by comparing
version numbers, or implicitly by inability to parse the new syntax.

bug#30786: Save text properties in desktop

[Juri Linkov]

Version: 27.0.50

> Currently I see no better way than in the aforementioned patch.
> At least it provides 100% guarantee of readability of the desktop file
> in the same version that wrote it.

Additionally I found out that savehist uses the same solution,
so it's consistent to do the same, pushed to master as 99de04e.