Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect breakpoint persistence from print-* vars #792

Merged
merged 2 commits into from
May 14, 2024

Conversation

basil-conto
Copy link
Contributor

@basil-conto basil-conto commented May 2, 2024

Summary

This PR comprises two commits:

  1. the first fixes the pitfall described below, reported by @cpitclaudel;
  2. the second looks at some opportunities for simplification spotted while looking around for places that do printing.

Pitfall

The output of printing functions (including prin1 and, by extension, format with a %s sequence) is affected by a set of output variables that both end-users and libraries can tweak.

This is fine in the more common use of these functions to format messages, but the problem is that the same functions are also the most convenient way to serialise/persist data.

Serialisation should be robust to non-default values of print-length and print-level in particular, as setting these to small numbers can lead to accidental data loss (and in some cases unreadable output).

For example, the following roundtrip returns "(0 ...)" instead of the expected "(0 1 2 3)":

(let (file)
  (unwind-protect
      (let ((data (number-sequence 0 3)))
        (setq file (make-temp-file "my-" nil ".eld"))
        (let ((print-length 1))
          (dap--persist file data))
        (with-temp-buffer
          (insert-file-contents file)
          (buffer-string)))
    (when file (delete-file file))))

Starting in Emacs 29, prin1 and co. gained a new optional argument precisely to protect against this problem. In the meantime, this PR ensures that print-length and print-level are set to their default values during serialisation.

The output of printing functions (including 'prin1' and, by
extension, 'format' with a '%s' sequence) is affected by a set of
output variables that both end-users and libraries can tweak.

This is fine in the more common use of these functions to format
messages, but the problem is that the same functions are also the
most convenient way to serialize/persist data.

Serialization should be robust to non-default values of
'print-length' and 'print-level' in particular, as setting these to
small numbers can lead to accidental data loss (and in some cases
unreadable output).

Starting in Emacs 29, 'prin1' and co. gained a new optional argument
precisely to protect against this problem.  In the meantime, this
patch ensures that 'print-length' and 'print-level' are set to their
default values during serialization.
'dap--persist': 'with-temp-file' creates a fresh buffer that is
already empty, so there is no need to erase it again.  Print
data-to-persist directly to output buffer, avoiding roundtrip via
temporary string.

'dap--print-to-output-buffer': modes are enabled with a 'nil' or
nonnegative numeric argument rather than with 't'; using
'turn-on-font-lock' avoids both this pitfall, as well as redundantly
reenabling the mode.  Use an 'inhibit-read-only' binding for
temporary read-only overrides, as modifications to
'buffer-read-only' will not be undone in case of a non-local exit.

'dap-debug-edit-template': Avoid temporary string roundtrip when
checking for an empty buffer.  Use 'insert-char' for repetitions of
the same character.  Remove redundant 'prin1-to-string' in arguments
to 'format'; it already uses 'prin1' for the '%s' sequence.
@yyoncho
Copy link
Member

yyoncho commented May 14, 2024

Thank you!

@yyoncho yyoncho merged commit 11431a2 into emacs-lsp:master May 14, 2024
10 of 12 checks passed
@basil-conto basil-conto deleted the blc/print branch May 14, 2024 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants