From 2c6bbe06c2f8966a519b4d222ed355ea64bbb1de Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Wed, 22 May 2024 19:36:47 -0500 Subject: [PATCH 01/23] Meta: v0.9-pre --- README.org | 4 ++++ plz.el | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/README.org b/README.org index d3d27d9..0a899f0 100644 --- a/README.org +++ b/README.org @@ -188,6 +188,10 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued :TOC: :depth 0 :END: +** 0.9-pre + +Nothing new yet. + ** 0.8 *Additions* diff --git a/plz.el b/plz.el index 57e247f..8c5246f 100644 --- a/plz.el +++ b/plz.el @@ -5,7 +5,7 @@ ;; Author: Adam Porter ;; Maintainer: Adam Porter ;; URL: https://github.com/alphapapa/plz.el -;; Version: 0.8 +;; Version: 0.9-pre ;; Package-Requires: ((emacs "26.3")) ;; Keywords: comm, network, http From a84e7dc405233aecb02961f35a78b19b4a5422ce Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Wed, 22 May 2024 19:35:56 -0500 Subject: [PATCH 02/23] Comment: Add FIXME --- plz.el | 1 + 1 file changed, 1 insertion(+) diff --git a/plz.el b/plz.el index 8c5246f..3d26cff 100644 --- a/plz.el +++ b/plz.el @@ -457,6 +457,7 @@ into the process buffer. (let ((filename (make-temp-file "plz-"))) (condition-case err (progn + ;; FIXME: Separate condition-case for writing the file. (write-region (point-min) (point-max) filename) (funcall then filename)) (file-already-exists From 5a9706c1c4fbe3d5312820d94ecf63290b1e88ab Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 02:04:49 -0500 Subject: [PATCH 03/23] Meta: Require Emacs 27.1 or later It's not practical to automatically test with Emacs 26.3 anymore (at least, not with the CI setup I have). plz might still work with it, or it might with minor changes, but I can't offer support for Emacs 26.3 anymore. Users of that version should probably use an earlier release of plz. --- README.org | 4 +++- plz.el | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.org b/README.org index 0a899f0..b313110 100644 --- a/README.org +++ b/README.org @@ -190,7 +190,9 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued ** 0.9-pre -Nothing new yet. +*Compatibility* + ++ The minimum supported Emacs version is now 27.1. (It is no longer practical to test ~plz~ with Emacs versions older than 27.1. For Emacs 26.3, an earlier version of ~plz~ may be used, or this version might be compatible, with or without minor changes, which the maintainer cannot offer support for.) ** 0.8 diff --git a/plz.el b/plz.el index 3d26cff..f75168c 100644 --- a/plz.el +++ b/plz.el @@ -6,7 +6,7 @@ ;; Maintainer: Adam Porter ;; URL: https://github.com/alphapapa/plz.el ;; Version: 0.9-pre -;; Package-Requires: ((emacs "26.3")) +;; Package-Requires: ((emacs "27.1")) ;; Keywords: comm, network, http ;; This file is part of GNU Emacs. From 46d0c54525a8d598960162d2717aecbab3d85b03 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 01:46:36 -0500 Subject: [PATCH 04/23] WIP See https://github.com/alphapapa/plz.el/issues/53. --- plz.el | 164 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 128 insertions(+), 36 deletions(-) diff --git a/plz.el b/plz.el index f75168c..09a81af 100644 --- a/plz.el +++ b/plz.el @@ -250,6 +250,70 @@ connection phase and waiting to receive the response (the \"--max-time\" argument to curl)." :type 'number) +;;;; Macros + +(require 'warnings) + +(cl-defmacro plz-debug (&rest args) + ;; Copied from `ement-debug' in Ement.el, which see. + "Display a debug warning showing the runtime value of ARGS. +The warning automatically includes the name of the containing +function, and it is only displayed if `warning-minimum-log-level' +is `:debug' at expansion time (otherwise the macro expands to a +call to `ignore' with ARGS and is eliminated by the +byte-compiler). When debugging, the form also returns nil so, +e.g. it may be used in a conditional in place of nil. + +Each of ARGS may be a string, which is displayed as-is, or a +symbol, the value of which is displayed prefixed by its name, or +a Lisp form, which is displayed prefixed by its first symbol. + +Before the actual ARGS arguments, you can write keyword +arguments, i.e. alternating keywords and values. The following +keywords are supported: + + :buffer BUFFER Name of buffer to pass to `display-warning'. + :level LEVEL Level passed to `display-warning', which see. + Default is :debug." + ;; TODO: Can we use a compiler macro to handle this more elegantly? + (pcase-let* ((fn-name (when byte-compile-current-buffer + (with-current-buffer byte-compile-current-buffer + ;; This is a hack, but a nifty one. + (save-excursion + (beginning-of-defun) + (cl-second (read (current-buffer))))))) + (plist-args (cl-loop while (keywordp (car args)) + collect (pop args) + collect (pop args))) + ((map (:buffer buffer) (:level level)) plist-args) + (level (or level :debug)) + (string (cl-loop for arg in args + concat (pcase arg + ((pred stringp) "%S ") + ((pred symbolp) + (concat (upcase (symbol-name arg)) ":%S ")) + ((pred listp) + (concat "(" (upcase (symbol-name (car arg))) + (pcase (length arg) + (1 ")") + (_ "...)")) + ":%S ")))))) + (if (eq :debug warning-minimum-log-level) + `(let ((fn-name ,(if fn-name + `',fn-name + ;; In an interpreted function: use `backtrace-frame' to get the + ;; function name (we have to use a little hackery to figure out + ;; how far up the frame to look, but this seems to work). + `(cl-loop for frame in (backtrace-frames) + for fn = (cl-second frame) + when (not (or (subrp fn) + (special-form-p fn) + (eq 'backtrace-frames fn))) + return (make-symbol (format "%s [interpreted]" fn)))))) + (display-warning fn-name (format ,string ,@args) ,level ,buffer) + nil) + `(ignore ,@args)))) + ;;;; Functions ;;;;; Public @@ -518,15 +582,19 @@ into the process buffer. (error "Process unexpectedly nil")) (while (accept-process-output process)) (while (accept-process-output stderr-process)) + (plz-debug (float-time) "BEFORE HACK" (process-buffer process)) (when (eq :plz-result (process-get process :plz-result)) + (plz-debug (float-time) "INSIDE HACK" (process-buffer process)) ;; HACK: Sentinel seems to not have been called: call it again. (Although ;; this is a hack, it seems to be a necessary one due to Emacs's process ;; handling.) See and ;; . - (plz--sentinel process "finished\n") + (plz--sentinel process "workaround") + (plz-debug (float-time) "INSIDE HACK, AFTER CALLING SENTINEL" (process-buffer process)) (when (eq :plz-result (process-get process :plz-result)) (error "Plz: NO RESULT FROM PROCESS:%S ARGS:%S" process rest))) + (plz-debug (float-time) "AFTER HACK" (process-buffer process)) ;; Sentinel seems to have been called: check the result. (pcase (process-get process :plz-result) ((and (pred plz-error-p) data) @@ -740,14 +808,28 @@ STATUS should be the process's event string (see info node `(elisp) Sentinels'). Calls `plz--respond' to process the HTTP response (directly for synchronous requests, or from a timer for asynchronous ones)." - (pcase status - ((or "finished\n" "killed\n" "interrupt\n" - (pred numberp) - (rx "exited abnormally with code " (group (1+ digit)))) - (let ((buffer (process-buffer process))) - (if (process-get process :plz-sync) - (plz--respond process buffer status) - (run-at-time 0 nil #'plz--respond process buffer status)))))) + (plz-debug (float-time) "BEFORE CONDITION" + process status (process-get process :plz-result)) + (if (eq :plz-result (process-get process :plz-result)) + ;; Result not yet set: call `plz--respond'. + (if (member (process-status process) '(run stop)) + (plz-debug "Doing nothing because:" (process-status process)) + ;; Process should have exited (otherwise we should do + ;; nothing). We check `process-status' because the STATUS + ;; variable might not be accurate (see "hack" in `plz'). + (pcase status + ((or "finished\n" "killed\n" "interrupt\n" "workaround" + (pred numberp) + (rx "exited abnormally with code " (group (1+ digit)))) + (let ((buffer (process-buffer process))) + (if (process-get process :plz-sync) + (plz--respond process buffer status) + (run-at-time 0 nil #'plz--respond process buffer status)))))) + ;; Result already set (likely indicating that Emacs did not call + ;; the sentinel when `accept-process-output' was called, so we are + ;; calling it from our "hack"): do nothing. + (plz-debug (float-time) ":PLZ-RESULT ALREADY CHANGED" + process status (process-get process :plz-result)))) (defun plz--respond (process buffer status) "Respond to HTTP response from PROCESS in BUFFER. @@ -760,11 +842,14 @@ argument passed to `plz--sentinel', which see." ;; "Respond" also means "to react to something," which is what this ;; does--react to receiving the HTTP response--and it's an internal ;; name, so why not. + (plz-debug (float-time) process status (process-status process) buffer) (unwind-protect - (with-current-buffer buffer - (pcase-exhaustive status - ((or 0 "finished\n") - ;; Curl exited normally: check HTTP status code. + (pcase-exhaustive (process-exit-status process) + (0 + ;; Curl exited normally: check HTTP status code. + (with-current-buffer buffer + ;; NOTE: We only switch to the process's buffer if curl + ;; exited successfully. (goto-char (point-min)) (plz--skip-proxy-headers) (while (plz--skip-redirect-headers)) @@ -784,29 +869,36 @@ argument passed to `plz--sentinel', which see." (let ((err (make-plz-error :response (plz--response)))) (pcase-exhaustive (process-get process :plz-else) (`nil (process-put process :plz-result err)) - ((and (pred functionp) fn) (funcall fn err))))))) - - ((or (and (pred numberp) code) - (rx "exited abnormally with code " (let code (group (1+ digit))))) - ;; Curl error. - (let* ((curl-exit-code (cl-typecase code - (string (string-to-number code)) - (number code))) - (curl-error-message (alist-get curl-exit-code plz-curl-errors)) - (err (make-plz-error :curl-error (cons curl-exit-code curl-error-message)))) - (pcase-exhaustive (process-get process :plz-else) - (`nil (process-put process :plz-result err)) - ((and (pred functionp) fn) (funcall fn err))))) - - ((and (or "killed\n" "interrupt\n") status) - ;; Curl process killed or interrupted. - (let* ((message (pcase status - ("killed\n" "curl process killed") - ("interrupt\n" "curl process interrupted"))) - (err (make-plz-error :message message))) - (pcase-exhaustive (process-get process :plz-else) - (`nil (process-put process :plz-result err)) - ((and (pred functionp) fn) (funcall fn err))))))) + ((and (pred functionp) fn) (funcall fn err)))))))) + ((and code (guard (<= 1 code 90))) + ;; Curl exited non-zero. + (let* ((curl-exit-code (cl-typecase code + (string (string-to-number code)) + (number code))) + (curl-error-message (alist-get curl-exit-code plz-curl-errors)) + (err (make-plz-error :curl-error (cons curl-exit-code curl-error-message)))) + (pcase-exhaustive (process-get process :plz-else) + (`nil (process-put process :plz-result err)) + ((and (pred functionp) fn) (funcall fn err))))) + ((and code (guard (not (<= 1 code 90)))) + ;; If we are here, it should mean that the curl process was + ;; killed or interrupted, and the code should be something + ;; not (<= 1 code 90). + (let* ((message (pcase status + ("killed\n" "curl process killed") + ("interrupt\n" "curl process interrupted") + (_ (format "Unexpected curl process status:%S code:%S. Please report this bug to the `plz' maintainer." status code)))) + (err (make-plz-error :message message))) + (pcase-exhaustive (process-get process :plz-else) + (`nil (process-put process :plz-result err)) + ((and (pred functionp) fn) (funcall fn err))))) + (code + ;; If we are here, something is really wrong. + (let* ((message (format "Unexpected curl process status:%S code:%S. Please report this bug to the `plz' maintainer." status code)) + (err (make-plz-error :message message))) + (pcase-exhaustive (process-get process :plz-else) + (`nil (process-put process :plz-result err)) + ((and (pred functionp) fn) (funcall fn err)))))) (when-let ((finally (process-get process :plz-finally))) (funcall finally)) (unless (or (process-get process :plz-sync) From 01b1ce77b83b5b675c7b45931a3f026258d3a8e9 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 01:59:58 -0500 Subject: [PATCH 05/23] Comment: Improve --- plz.el | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/plz.el b/plz.el index 09a81af..73e9d70 100644 --- a/plz.el +++ b/plz.el @@ -811,23 +811,26 @@ for asynchronous ones)." (plz-debug (float-time) "BEFORE CONDITION" process status (process-get process :plz-result)) (if (eq :plz-result (process-get process :plz-result)) - ;; Result not yet set: call `plz--respond'. + ;; Result not yet set: check process status (we call + ;; `process-status' because the STATUS argument might not be + ;; accurate--see "hack" in `plz'). (if (member (process-status process) '(run stop)) + ;; Process still alive: do nothing. (plz-debug "Doing nothing because:" (process-status process)) - ;; Process should have exited (otherwise we should do - ;; nothing). We check `process-status' because the STATUS - ;; variable might not be accurate (see "hack" in `plz'). + ;; Process appears to be dead: check STATUS argument. (pcase status ((or "finished\n" "killed\n" "interrupt\n" "workaround" (pred numberp) (rx "exited abnormally with code " (group (1+ digit)))) + ;; STATUS seems okay: call `plz--respond'. (let ((buffer (process-buffer process))) (if (process-get process :plz-sync) (plz--respond process buffer status) (run-at-time 0 nil #'plz--respond process buffer status)))))) ;; Result already set (likely indicating that Emacs did not call ;; the sentinel when `accept-process-output' was called, so we are - ;; calling it from our "hack"): do nothing. + ;; either being called from our "hack", or being called a second + ;; time, after `plz' returned): do nothing. (plz-debug (float-time) ":PLZ-RESULT ALREADY CHANGED" process status (process-get process :plz-result)))) From d9644302c78101b50402d8448759893b5fd04759 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 02:08:04 -0500 Subject: [PATCH 06/23] Tidy: Docstring Checkdoooooooc! --- plz.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plz.el b/plz.el index 73e9d70..72b8bea 100644 --- a/plz.el +++ b/plz.el @@ -256,7 +256,7 @@ connection phase and waiting to receive the response (the (cl-defmacro plz-debug (&rest args) ;; Copied from `ement-debug' in Ement.el, which see. - "Display a debug warning showing the runtime value of ARGS. + "Display a debug warning showing the run-time value of ARGS. The warning automatically includes the name of the containing function, and it is only displayed if `warning-minimum-log-level' is `:debug' at expansion time (otherwise the macro expands to a From be1d63c7d8fe4763b3bfb943dd48063d731e7aed Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 22:18:16 -0500 Subject: [PATCH 07/23] Docs: Update changelog --- README.org | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.org b/README.org index b313110..6e33a90 100644 --- a/README.org +++ b/README.org @@ -194,6 +194,10 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued + The minimum supported Emacs version is now 27.1. (It is no longer practical to test ~plz~ with Emacs versions older than 27.1. For Emacs 26.3, an earlier version of ~plz~ may be used, or this version might be compatible, with or without minor changes, which the maintainer cannot offer support for.) +*Fixes* + ++ Improve workaround for Emacs's process sentinel-related issues. (Don't try to process response a second time if Emacs calls the sentinel after ~plz~ has returned for a synchronous request. See [[https://github.com/alphapapa/plz.el/issues/53][#53]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for extensive help debugging, and to [[https://ushin.org/][USHIN]] for sponsoring some of this work.) + ** 0.8 *Additions* From e077c706a240abfdc4ddee71d677668b2d35f159 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 22:22:29 -0500 Subject: [PATCH 08/23] Docs: Update USHIN references To their preferred format. --- README.org | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.org b/README.org index 6e33a90..042dd4d 100644 --- a/README.org +++ b/README.org @@ -275,13 +275,13 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued ** 0.4 *Additions* -+ Support for HTTP ~HEAD~ requests. (Thanks to [[https://ushin.org/][USHIN, Inc.]] for sponsoring.) ++ Support for HTTP ~HEAD~ requests. (Thanks to [[https://ushin.org/][USHIN]] for sponsoring.) *Changes* -+ Allow sending ~POST~ and ~PUT~ requests without bodies. ([[https://github.com/alphapapa/plz.el/issues/16][#16]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for reporting. Thanks to [[https://ushin.org/][USHIN, Inc.]] for sponsoring.) ++ Allow sending ~POST~ and ~PUT~ requests without bodies. ([[https://github.com/alphapapa/plz.el/issues/16][#16]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for reporting. Thanks to [[https://ushin.org/][USHIN]] for sponsoring.) *Fixes* -+ All 2xx HTTP status codes are considered successful. ([[https://github.com/alphapapa/plz.el/issues/17][#17]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for reporting. Thanks to [[https://ushin.org/][USHIN, Inc.]] for sponsoring.) ++ All 2xx HTTP status codes are considered successful. ([[https://github.com/alphapapa/plz.el/issues/17][#17]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for reporting. Thanks to [[https://ushin.org/][USHIN]] for sponsoring.) + Errors are signaled with error data correctly. *Internal* From b3e764c36a2d82b3b788bf9688fc69488f661d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krzywkowski?= Date: Thu, 23 May 2024 10:25:54 +0200 Subject: [PATCH 09/23] Prevent yes-or-no-p query when killing plz process buffers --- plz.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plz.el b/plz.el index 72b8bea..7ff57f2 100644 --- a/plz.el +++ b/plz.el @@ -467,9 +467,9 @@ into the process buffer. ;; default-directory has since been removed). It's unclear what the best ;; directory is, but this seems to make sense, and it should still exist. temporary-file-directory) - (process-buffer (generate-new-buffer " *plz-request-curl*")) + (process-buffer (generate-new-buffer " *plz-request-curl*" t)) (stderr-process (make-pipe-process :name "plz-request-curl-stderr" - :buffer (generate-new-buffer " *plz-request-curl-stderr*") + :buffer (generate-new-buffer " *plz-request-curl-stderr*" t) :noquery t :sentinel #'plz--stderr-sentinel)) (process (make-process :name "plz-request-curl" From f2b1ee045b476461851d60ed6f50e6b5b546b9d1 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 22:26:23 -0500 Subject: [PATCH 10/23] Docs: Update changelog --- README.org | 1 + 1 file changed, 1 insertion(+) diff --git a/README.org b/README.org index 042dd4d..0e6e2e0 100644 --- a/README.org +++ b/README.org @@ -197,6 +197,7 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued *Fixes* + Improve workaround for Emacs's process sentinel-related issues. (Don't try to process response a second time if Emacs calls the sentinel after ~plz~ has returned for a synchronous request. See [[https://github.com/alphapapa/plz.el/issues/53][#53]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for extensive help debugging, and to [[https://ushin.org/][USHIN]] for sponsoring some of this work.) ++ Inhibit buffer hooks when calling ~generate-new-buffer~ (as extra protection against "kill buffer?" prompts in case of errors). (See [[https://github.com/alphapapa/plz.el/pull/52][#52]]. Thanks to [[https://github.com/mkcms][Michał Krzywkowski]].) ** 0.8 From 4cfd78294df69e13656ce5098eaa552615af29a6 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 22:30:37 -0500 Subject: [PATCH 11/23] Tests: Test on Emacs 29.1, 29.2, and 29.3 --- .github/workflows/test.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0f991e4..bc2953a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -51,6 +51,9 @@ jobs: - 27.2 - 28.1 - 28.2 + - 29.1 + - 29.2 + - 29.3 - snapshot steps: - uses: purcell/setup-emacs@master From 3e85bad7b3ef19065306e889f8c9e188d287009c Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 22:30:54 -0500 Subject: [PATCH 12/23] Tests: Remove obsolete 26.3-related code and comment --- .github/workflows/test.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bc2953a..0aa1184 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -41,12 +41,6 @@ jobs: fail-fast: false matrix: emacs_version: - # FIXME: The 26.3 test fails to initialize with the error - # "Package ‘emacs-27.1’ is unavailable", which happens just - # after "Package refresh done". Not sure what the cause is. - # But I don't want the whole suite marked as failing because - # of 26.3 right now, so commenting it out. - # - 26.3 - 27.1 - 27.2 - 28.1 From 2534262975f8ba731001f7283e0a6229c45b1969 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 22:39:57 -0500 Subject: [PATCH 13/23] Fix: Alias generate-new-buffer for Emacs <28.1 --- plz.el | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/plz.el b/plz.el index 7ff57f2..d60ede4 100644 --- a/plz.el +++ b/plz.el @@ -314,6 +314,16 @@ keywords are supported: nil) `(ignore ,@args)))) +;;;; Compatibility + +(defalias 'plz--generate-new-buffer + (if (version< emacs-version "28.1") + (lambda (name &optional _inhibit-buffer-hooks) + "Call `generate-new-buffer' with NAME. +Compatibility function for Emacs versions <28.1." + (generate-new-buffer name)) + #'generate-new-buffer)) + ;;;; Functions ;;;;; Public @@ -467,9 +477,9 @@ into the process buffer. ;; default-directory has since been removed). It's unclear what the best ;; directory is, but this seems to make sense, and it should still exist. temporary-file-directory) - (process-buffer (generate-new-buffer " *plz-request-curl*" t)) + (process-buffer (plz--generate-new-buffer " *plz-request-curl*" t)) (stderr-process (make-pipe-process :name "plz-request-curl-stderr" - :buffer (generate-new-buffer " *plz-request-curl-stderr*" t) + :buffer (plz--generate-new-buffer " *plz-request-curl-stderr*" t) :noquery t :sentinel #'plz--stderr-sentinel)) (process (make-process :name "plz-request-curl" From 6fbfc11d6ee4a56ad0b5f4d212047ae400617028 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sat, 25 May 2024 22:44:45 -0500 Subject: [PATCH 14/23] Docs: Update changelog about testing Emacs versions --- README.org | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.org b/README.org index 0e6e2e0..84a7111 100644 --- a/README.org +++ b/README.org @@ -199,6 +199,10 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued + Improve workaround for Emacs's process sentinel-related issues. (Don't try to process response a second time if Emacs calls the sentinel after ~plz~ has returned for a synchronous request. See [[https://github.com/alphapapa/plz.el/issues/53][#53]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for extensive help debugging, and to [[https://ushin.org/][USHIN]] for sponsoring some of this work.) + Inhibit buffer hooks when calling ~generate-new-buffer~ (as extra protection against "kill buffer?" prompts in case of errors). (See [[https://github.com/alphapapa/plz.el/pull/52][#52]]. Thanks to [[https://github.com/mkcms][Michał Krzywkowski]].) +*Development* + ++ ~plz~ is now automatically tested against Emacs versions 27.1, 27.2, 28.1, 28.2, 29.1, 29.2, 29.3, and a recent snapshot of the ~master~ branch (adding 29.2 and 29.3). + ** 0.8 *Additions* From 831348a25809338f31d22ccc5318b1aa4546588f Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sun, 26 May 2024 00:44:35 -0500 Subject: [PATCH 15/23] Comment: Add TODO --- plz.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plz.el b/plz.el index d60ede4..3c4b4aa 100644 --- a/plz.el +++ b/plz.el @@ -244,6 +244,8 @@ This limits how long the connection phase may last (the :type 'number) (defcustom plz-timeout 60 + ;; TODO(v0.9): Remove the `plz-timeout' option. Requests shouldn't + ;; have a "max-time" timeout by default, anyway. "Default request timeout in seconds. This limits how long an entire request may take, including the connection phase and waiting to receive the response (the From 0ebbe24e8f0f388d48f4e6375cdabc92f5e93c38 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sun, 26 May 2024 01:09:53 -0500 Subject: [PATCH 16/23] Fix: Require map For the PCASE MAP form in PLZ-DEBUG. --- plz.el | 1 + 1 file changed, 1 insertion(+) diff --git a/plz.el b/plz.el index 3c4b4aa..be61e1b 100644 --- a/plz.el +++ b/plz.el @@ -98,6 +98,7 @@ ;;;; Requirements (require 'cl-lib) +(require 'map) (require 'rx) (require 'subr-x) From e802cf270b58fab29b83dc78692af4865b789257 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sun, 26 May 2024 01:15:28 -0500 Subject: [PATCH 17/23] Remove: (plz-timeout) Option Specifying a default value for PLZ's :TIMEOUT argument was a mistake. --- README.org | 4 ++++ plz.el | 13 ++----------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/README.org b/README.org index 84a7111..47c16ab 100644 --- a/README.org +++ b/README.org @@ -194,6 +194,10 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued + The minimum supported Emacs version is now 27.1. (It is no longer practical to test ~plz~ with Emacs versions older than 27.1. For Emacs 26.3, an earlier version of ~plz~ may be used, or this version might be compatible, with or without minor changes, which the maintainer cannot offer support for.) +*Changes* + ++ Option ~plz-timeout~ is removed. (It was the default value for ~plz~'s ~:timeout~ argument, which is passed to Curl as its ~--max-time~ argument, limiting the total duration of a request operation. This argument should be unset by default, because larger or slower downloads might not finish within a certain duration, and it is surprising to the user to have this option set by default, potentially causing requests to timeout unnecessarily.) + *Fixes* + Improve workaround for Emacs's process sentinel-related issues. (Don't try to process response a second time if Emacs calls the sentinel after ~plz~ has returned for a synchronous request. See [[https://github.com/alphapapa/plz.el/issues/53][#53]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for extensive help debugging, and to [[https://ushin.org/][USHIN]] for sponsoring some of this work.) diff --git a/plz.el b/plz.el index be61e1b..f5d8d5b 100644 --- a/plz.el +++ b/plz.el @@ -244,15 +244,6 @@ This limits how long the connection phase may last (the \"--connect-timeout\" argument to curl)." :type 'number) -(defcustom plz-timeout 60 - ;; TODO(v0.9): Remove the `plz-timeout' option. Requests shouldn't - ;; have a "max-time" timeout by default, anyway. - "Default request timeout in seconds. -This limits how long an entire request may take, including the -connection phase and waiting to receive the response (the -\"--max-time\" argument to curl)." - :type 'number) - ;;;; Macros (require 'warnings) @@ -331,10 +322,10 @@ Compatibility function for Emacs versions <28.1." ;;;;; Public -(cl-defun plz (method url &rest rest &key headers body else filter finally noquery +(cl-defun plz (method url &rest rest &key headers body else filter finally noquery timeout (as 'string) (then 'sync) (body-type 'text) (decode t decode-s) - (connect-timeout plz-connect-timeout) (timeout plz-timeout)) + (connect-timeout plz-connect-timeout)) "Request METHOD from URL with curl. Return the curl process object or, for a synchronous request, the selected result. From 106ef828ea44a5802df8c064281848d7a5513ffd Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sun, 26 May 2024 01:16:56 -0500 Subject: [PATCH 18/23] Docs: (plz) Improve docstring on TIMEOUT argument --- plz.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plz.el b/plz.el index f5d8d5b..56ae9d4 100644 --- a/plz.el +++ b/plz.el @@ -393,8 +393,8 @@ THEN or ELSE, as appropriate. For synchronous requests, this argument is ignored. CONNECT-TIMEOUT and TIMEOUT are a number of seconds that limit -how long it takes to connect to a host and to receive a response -from a host, respectively. +how long it takes to connect to a host and to receive a complete +response from a host, respectively. NOQUERY is passed to `make-process', which see. From ff8a1e9aaa5942e9b1b67219d2a5893841b27112 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Sun, 26 May 2024 01:35:29 -0500 Subject: [PATCH 19/23] Change: (plz) Pass filenames to curl when downloading as file --- README.org | 1 + plz.el | 91 +++++++++++++++++++++++++++--------------------------- 2 files changed, 47 insertions(+), 45 deletions(-) diff --git a/README.org b/README.org index 47c16ab..f70586c 100644 --- a/README.org +++ b/README.org @@ -197,6 +197,7 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued *Changes* + Option ~plz-timeout~ is removed. (It was the default value for ~plz~'s ~:timeout~ argument, which is passed to Curl as its ~--max-time~ argument, limiting the total duration of a request operation. This argument should be unset by default, because larger or slower downloads might not finish within a certain duration, and it is surprising to the user to have this option set by default, potentially causing requests to timeout unnecessarily.) ++ Using arguments ~:as 'file~ or ~:as '(file FILENAME)~ now passes the filename to Curl, allowing it to write the data to the file itself (rather than receiving the data into an Emacs buffer and then writing it to a file. This improves performance when downloading large files, significantly reducing Emacs's CPU and memory usage). *Fixes* diff --git a/plz.el b/plz.el index 56ae9d4..4bcae37 100644 --- a/plz.el +++ b/plz.el @@ -419,7 +419,8 @@ into the process buffer. ;; the empty string. See . ;; TODO: Handle "100 Continue" responses and remove this workaround. (push (cons "Expect" "") headers) - (let* ((data-arg (pcase-exhaustive body-type + (let* (filename + (data-arg (pcase-exhaustive body-type ('binary "--data-binary") ('text "--data"))) (curl-command-line-args (append plz-curl-default-args @@ -443,21 +444,49 @@ into the process buffer. ;; method. (pcase method ('get - (list (cons "--dump-header" "-"))) + (append (list (cons "--dump-header" "-")) + (pcase as + ('file + (setf filename (make-temp-file "plz-")) + (list (cons "--output" filename))) + (`(file ,(and (pred stringp) as-filename)) + (when (file-exists-p as-filename) + (error "File exists, will not overwrite: %S" as-filename)) + (setf filename as-filename) + (list (cons "--output" filename)))))) ((or 'put 'post) - (list (cons "--dump-header" "-") - (cons "--request" (upcase (symbol-name method))) - ;; It appears that this must be the last argument - ;; in order to pass data on the rest of STDIN. - (pcase body - (`(file ,filename) - ;; Use `expand-file-name' because curl doesn't - ;; expand, e.g. "~" into "/home/...". - (cons "--upload-file" (expand-file-name filename))) - (_ (cons data-arg "@-"))))) + (append (list (cons "--dump-header" "-") + (cons "--request" (upcase (symbol-name method)))) + (pcase as + ('file + (setf filename (make-temp-file "plz-")) + (list (cons "--output" filename))) + (`(file ,(and (pred stringp) as-filename)) + (when (file-exists-p as-filename) + (error "File exists, will not overwrite: %S" as-filename)) + (setf filename as-filename) + (list (cons "--output" filename)))) + (list + ;; It appears that this must be the last argument + ;; in order to pass data on the rest of STDIN. + (pcase body + (`(file ,filename) + ;; Use `expand-file-name' because curl doesn't + ;; expand, e.g. "~" into "/home/...". + (cons "--upload-file" (expand-file-name filename))) + (_ (cons data-arg "@-")))))) ('delete - (list (cons "--dump-header" "-") - (cons "--request" (upcase (symbol-name method))))) + (append (list (cons "--dump-header" "-") + (cons "--request" (upcase (symbol-name method)))) + (pcase as + ('file + (setf filename (make-temp-file "plz-")) + (list (cons "--output" filename))) + (`(file ,(and (pred stringp) as-filename)) + (when (file-exists-p as-filename) + (error "File exists, will not overwrite: %S" as-filename)) + (setf filename as-filename) + (list (cons "--output" filename)))))) ('head (list (cons "--head" "") (cons "--request" "HEAD")))))) @@ -520,39 +549,11 @@ into the process buffer. (make-plz-error :message (format "response is nil for buffer:%S buffer-string:%S" process-buffer (buffer-string))))))) ('file (lambda () - (set-buffer-multibyte nil) - (plz--narrow-to-body) - (let ((filename (make-temp-file "plz-"))) - (condition-case err - (progn - ;; FIXME: Separate condition-case for writing the file. - (write-region (point-min) (point-max) filename) - (funcall then filename)) - (file-already-exists - (funcall then (make-plz-error :message (format "error while writing to file %S: %S" filename err)))) - ;; In case of an error writing to the file, delete the temp file - ;; and signal the error. Ignore any errors encountered while - ;; deleting the file, which would obscure the original error. - (error (ignore-errors - (delete-file filename)) - (funcall then (make-plz-error :message (format "error while writing to file %S: %S" filename err)))))))) + (funcall then filename))) (`(file ,(and (pred stringp) filename)) + ;; This requires a separate clause due to the FILENAME binding. (lambda () - (set-buffer-multibyte nil) - (plz--narrow-to-body) - (condition-case err - (progn - (write-region (point-min) (point-max) filename nil nil nil 'excl) - (funcall then filename)) - (file-already-exists - (funcall then (make-plz-error :message (format "error while writing to file %S: %S" filename err)))) - ;; Since we are creating the file, it seems sensible to delete it in case of an - ;; error while writing to it (e.g. a disk-full error). And we ignore any errors - ;; encountered while deleting the file, which would obscure the original error. - (error (ignore-errors - (when (file-exists-p filename) - (delete-file filename))) - (funcall then (make-plz-error :message (format "error while writing to file %S: %S" filename err))))))) + (funcall then filename))) ((pred functionp) (lambda () (let ((coding-system (or (plz--coding-system) 'utf-8))) (plz--narrow-to-body) From fee8fb47ec1185a87fb263eb49de95f45ee73a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krzywkowski?= Date: Mon, 10 Jun 2024 14:16:14 -0500 Subject: [PATCH 20/23] Change: (plz--kill-buffer) Avoid prompting on Emacs <28 See and . Co-authored-by: Adam Porter --- plz.el | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/plz.el b/plz.el index 4bcae37..01299a2 100644 --- a/plz.el +++ b/plz.el @@ -614,8 +614,8 @@ into the process buffer. ;; The AS function returned a value: return it. else))) (unless (eq as 'buffer) - (kill-buffer process-buffer)) - (kill-buffer (process-buffer stderr-process))) + (plz--kill-buffer process-buffer)) + (plz--kill-buffer (process-buffer stderr-process))) ;; Async request: return the process object. process))) @@ -911,7 +911,7 @@ argument passed to `plz--sentinel', which see." (funcall finally)) (unless (or (process-get process :plz-sync) (eq 'buffer (process-get process :plz-as))) - (kill-buffer buffer)))) + (plz--kill-buffer buffer)))) (defun plz--stderr-sentinel (process status) "Sentinel for STDERR buffer. @@ -920,7 +920,14 @@ Arguments are PROCESS and STATUS (ok, checkdoc?)." ((or "finished\n" "killed\n" "interrupt\n" (pred numberp) (rx "exited abnormally with code " (1+ digit))) - (kill-buffer (process-buffer process))))) + (plz--kill-buffer (process-buffer process))))) + +(defun plz--kill-buffer (&optional buffer) + "Kill BUFFER unconditionally, without asking for confirmation. +Binds `kill-buffer-query-functions' to nil." + ;; TODO(emacs-28): Remove this workaround when requiring Emacs 28+. + (let (kill-buffer-query-functions) + (kill-buffer buffer))) ;;;;;; HTTP Responses From de8554599a457779240b3fe9f9ee1333e85368ef Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Mon, 10 Jun 2024 14:18:50 -0500 Subject: [PATCH 21/23] Docs: Update changelog --- README.org | 1 + 1 file changed, 1 insertion(+) diff --git a/README.org b/README.org index f70586c..4413f60 100644 --- a/README.org +++ b/README.org @@ -203,6 +203,7 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued + Improve workaround for Emacs's process sentinel-related issues. (Don't try to process response a second time if Emacs calls the sentinel after ~plz~ has returned for a synchronous request. See [[https://github.com/alphapapa/plz.el/issues/53][#53]]. Thanks to [[https://github.com/josephmturner][Joseph Turner]] for extensive help debugging, and to [[https://ushin.org/][USHIN]] for sponsoring some of this work.) + Inhibit buffer hooks when calling ~generate-new-buffer~ (as extra protection against "kill buffer?" prompts in case of errors). (See [[https://github.com/alphapapa/plz.el/pull/52][#52]]. Thanks to [[https://github.com/mkcms][Michał Krzywkowski]].) + - Avoid "kill buffer?" prompts in case of errors on Emacs versions before 28. (See [[https://github.com/alphapapa/plz.el/pull/52][#52]] and [[https://github.com/alphapapa/plz.el/issues/57][#57]]. Thanks to [[https://github.com/mkcms][Michał Krzywkowski]].) *Development* From 4683adb5315b207648a68dd3d75f659c2748e8be Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Mon, 10 Jun 2024 14:20:53 -0500 Subject: [PATCH 22/23] Comment/Docs: Retarget error changes for v0.10, etc. --- plz.el | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/plz.el b/plz.el index 01299a2..18a0f3d 100644 --- a/plz.el +++ b/plz.el @@ -46,13 +46,14 @@ ;;;; Usage: -;; FIXME(v0.8): Remove the following note. +;; FIXME(v0.10): Remove the following note. -;; NOTE: In v0.8 of plz, only one error will be signaled: `plz-error'. -;; The existing errors, `plz-curl-error' and `plz-http-error', inherit -;; from `plz-error' to allow applications to update their code while -;; using v0.7 (i.e. any `condition-case' forms should now handle only -;; `plz-error', not the other two). +;; NOTE: In a future version of plz, only one error will be signaled: +;; `plz-error'. The existing errors, `plz-curl-error' and +;; `plz-http-error', inherit from `plz-error' to allow applications to +;; update their code while using earlier versions (i.e. any +;; `condition-case' forms should now handle only `plz-error', not the +;; other two). ;; Call function `plz' to make an HTTP request. Its docstring ;; explains its arguments. `plz' also supports other HTTP methods, @@ -382,11 +383,12 @@ structure. If ELSE is nil, a `plz-curl-error' or `plz-error' structure as the error data. For synchronous requests, this argument is ignored. -NOTE: In v0.8 of `plz', only one error will be signaled: -`plz-error'. The existing errors, `plz-curl-error' and +NOTE: In a future version of `plz', only one error will be +signaled: `plz-error'. The existing errors, `plz-curl-error' and `plz-http-error', inherit from `plz-error' to allow applications -to update their code while using v0.7 (i.e. any `condition-case' -forms should now handle only `plz-error', not the other two). +to update their code while using earlier versions (i.e. any +`condition-case' forms should now handle only `plz-error', not +the other two). FINALLY is an optional function called without argument after THEN or ELSE, as appropriate. For synchronous requests, this @@ -408,8 +410,8 @@ FILTER function should at least insert output up to the HTTP body into the process buffer. \(To silence checkdoc, we mention the internal argument REST.)" - ;; FIXME(v0.8): Remove the note about error changes from the docstring. - ;; FIXME(v0.8): Update error signals in docstring. + ;; FIXME(v0.10): Remove the note about error changes from the docstring. + ;; FIXME(v0.10): Update error signals in docstring. (declare (indent defun)) (setf decode (if (and decode-s (not decode)) nil decode)) @@ -607,7 +609,7 @@ into the process buffer. ;; into a `plz-error' struct: re-signal the error here, ;; outside of the sentinel. (if (plz-error-response data) - ;; FIXME(v0.8): Signal only plz-error. + ;; FIXME(v0.10): Signal only plz-error. (signal 'plz-http-error (list "HTTP error" data)) (signal 'plz-curl-error (list "Curl error" data)))) (else @@ -737,7 +739,7 @@ QUEUE should be a `plz-queue' structure." (setf (plz-queue-active queue) (delq request (plz-queue-active queue))) (plz-run queue)))) (else (lambda (arg) - ;; FIXME(v0.8): This should be done in `plz-queue' because + ;; FIXME(v0.10): This should be done in `plz-queue' because ;; `plz-clear' will call the second queued-request's ELSE ;; before it can be set by `plz-run'. (unwind-protect @@ -870,7 +872,7 @@ argument passed to `plz--sentinel', which see." ;; TODO: If using ":as 'response", the HTTP response ;; should be passed to the THEN function, regardless ;; of the status code. Only for curl errors should - ;; the ELSE function be called. (Maybe in v0.8.) + ;; the ELSE function be called. (Maybe in v0.10.) ;; Any other status code is considered unsuccessful ;; (for now, anyway). From 399ad3e1aafa7b86c1ce0cfab8bcf2e09cb0e956 Mon Sep 17 00:00:00 2001 From: Adam Porter Date: Mon, 10 Jun 2024 14:21:47 -0500 Subject: [PATCH 23/23] Release: v0.9 --- README.org | 2 +- plz.el | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.org b/README.org index 4413f60..bc7b286 100644 --- a/README.org +++ b/README.org @@ -188,7 +188,7 @@ You may also clear a queue with ~plz-clear~, which cancels any active or queued :TOC: :depth 0 :END: -** 0.9-pre +** 0.9 *Compatibility* diff --git a/plz.el b/plz.el index 18a0f3d..49b37ad 100644 --- a/plz.el +++ b/plz.el @@ -5,7 +5,7 @@ ;; Author: Adam Porter ;; Maintainer: Adam Porter ;; URL: https://github.com/alphapapa/plz.el -;; Version: 0.9-pre +;; Version: 0.9 ;; Package-Requires: ((emacs "27.1")) ;; Keywords: comm, network, http