* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping @ 2023-09-13 2:24 sbaugh 2023-09-13 2:30 ` sbaugh 0 siblings, 1 reply; 59+ messages in thread From: sbaugh @ 2023-09-13 2:24 UTC (permalink / raw) To: 65902 The emacsclient-mail.desktop file fails on my system with the following error: "$1": -c: line 2: unexpected EOF while looking for matching `)' My subsequent mail will fix this also vastly simplify emacsclient-mail.desktop by adding a new feature to emacsclient to avoid the need for complex escaping. In GNU Emacs 29.0.92 (build 68, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw3d scroll bars) of 2023-07-15 built on earth Repository revision: 36d3460f9f6064c03fd88e0c969c8e4f4d645235 Repository branch: emacs-29 Windowing system distributor 'The X.Org Foundation', version 11.0.12101008 System Description: NixOS 23.05 (Stoat) Configured using: 'configure --cache-file=config.cache --with-x-toolkit=lucid --with-tree-sitter --with-xinput2 CC=gcc PKG_CONFIG=pkg-config PKG_CONFIG_PATH=/nix/store/s3r15m8wbl4wqk4khqlf41ikryhjm1bi-file-5.44-dev/lib/pkgconfig:/nix/store/f9jbn419h46c78z1pi49yn9a8742b0ql-gnutls-3.8.0-dev/lib/pkgconfig:/nix/store/knq0pv08wm4dins7m4jh0n7cv7pjvdjr-nettle-3.9.1-dev/lib/pkgconfig:/nix/store/dy8p07vrrhdgpnl45xz9c0k0didbikdh-gmp-with-cxx-6.2.1-dev/lib/pkgconfig:/nix/store/6hkdabzyqhyq5ypq4c9b2cibr1d1zg1s-harfbuzz-7.3.0-dev/lib/pkgconfig:/nix/store/hyns944pqgblw4icskylvlpm5krmfvcr-graphite2-1.3.14-dev/lib/pkgconfig:/nix/store/08cdp9vgvy023ysfa2y01gzsm2jv6phx-jansson-2.14/lib/pkgconfig:/nix/store/nqlbk40lh7igs61l77dwgdkn8dc2akcm-libxml2-2.10.4-dev/lib/pkgconfig:/nix/store/b3axl73v3yvqqx7g47njqb5wzxvm280p-zlib-1.2.13-dev/lib/pkgconfig:/nix/store/3f2rc4inlcxmq11718qmz94v2rpybw70-ncurses-6.4-dev/lib/pkgconfig:/nix/store/bxy745kyb1fwhpfkiaaz3wgvpkpvwcpq-dbus-1.14.8-dev/lib/pkgconfig:/nix/store/9714v7c4cgpm4yqcyqk6n9xw9iq3a1bs-expat-2.5.0-dev/lib/pkgconfig:/nix/store/zzi7pcadidqh798yddxv6pwdbwpkikma-libselinux-3.3-dev/lib/pkgconfig:/nix/store/w14j7y5nl14vy4ikcivss35jmrqq3fxj-libotf-0.9.16-dev/lib/pkgconfig:/nix/store/arhk7hsch4scyv6m24fw03yq6wq5wbbx-m17n-lib-1.8.2/lib/pkgconfig:/nix/store/1jbbrny8xcjb68lb5m30cvxycfkyhvsv-sqlite-3.42.0-dev/lib/pkgconfig:/nix/store/5vx779yqkxaysv48gicwlgv0ippbrhc4-systemd-253.5-dev/lib/pkgconfig:/nix/store/5vx779yqkxaysv48gicwlgv0ippbrhc4-systemd-253.5-dev/share/pkgconfig:/nix/store/djifahvk3qp06ssqxv6gy1ixdnnypr9s-tree-sitter-0.20.8/lib/pkgconfig:/nix/store/74aasy1d2r5y27zn68cs1rxwy1llzn05-libwebp-1.3.0/lib/pkgconfig:/nix/store/8sk7bp89iwb4gw96fq6xakb6lcy2x52n-Xaw3d-1.6.3/lib/pkgconfig:/nix/store/ppvb3ha8148am3ajnzxnm6i3ri38c01n-libXmu-1.1.3-dev/lib/pkgconfig:/nix/store/jyxf8cjbj3nzh00x48nfram79i63chdi-libX11-1.8.6-dev/lib/pkgconfig:/nix/store/zk9v0nr5zdfi1ybkhcfifmxsng7hfl23-xorgproto-2021.5/share/pkgconfig:/nix/store/3q1k18v8aa6mxs538bha4ry0mp3m321l-libxcb-1.14-dev/lib/pkgconfig:/nix/store/hcscz68zvfk1skyb25wrnha959f6hhrc-libXt-1.2.1-dev/lib/pkgconfig:/nix/store/kl55wj6qc3v481jsgvzm5w2csnhm84zf-libSM-1.2.3-dev/lib/pkgconfig:/nix/store/s3f67kvsn55rxp2rc98xv0hkq364yci1-libICE-1.0.10-dev/lib/pkgconfig:/nix/store/rsw4ri8025jgln8vpsrmg82bzgbcw3zr-cairo-1.16.0-dev/lib/pkgconfig:/nix/store/jir0rqbcy0d9qr9kf5cwf2yphql4ykyw-fontconfig-2.14.2-dev/lib/pkgconfig:/nix/store/n2g3xblaz1k4civv1z6hhm1nsmp3m17p-freetype-2.13.0-dev/lib/pkgconfig:/nix/store/isbmyzm2shmp0wsjr4cy45v2i58h2zvw-bzip2-1.0.8-dev/lib/pkgconfig:/nix/store/bl2qwy78jr2sqm260imgxmd5dzhjqvag-brotli-1.0.9-dev/lib/pkgconfig:/nix/store/z96jh9ag5b3565lwwb5chjb9bfp5i2qv-libpng-apng-1.6.39-dev/lib/pkgconfig:/nix/store/jjd4z18grhky6lh8n463v648nnf5628b-pixman-0.42.2/lib/pkgconfig:/nix/store/qd14wrazwcspjv3q65vgh35pl7b8nifq-libXext-1.3.4-dev/lib/pkgconfig:/nix/store/gj8i21xx87ip9b971j2d1m0rmrzyhbir-libXau-1.0.9-dev/lib/pkgconfig:/nix/store/4gpinwwdqhi927xkrfpr1hvdd56baxgk-libXrender-0.9.10-dev/lib/pkgconfig:/nix/store/d1jbygs6hcn6dysk706i9zf07yd18wmr-xcb-util-0.4.1-dev/lib/pkgconfig:/nix/store/hdc4ika0mb1cv0cf6dchwxbr004rc50i-glib-2.76.3-dev/lib/pkgconfig:/nix/store/wxyh848a6xcqy2v8727vcwspri53pqwi-libffi-3.4.4-dev/lib/pkgconfig:/nix/store/42jx72681qzliic0xsjhvx24cil2gapk-libGL-1.6.0-dev/lib/pkgconfig:/nix/store/b9lmdkxpvgkj6zc956fvhshzisqpi767-libglvnd-1.6.0-dev/lib/pkgconfig:/nix/store/gff29sbhg1gcw969mpm5rb693kj5v18w-libXaw-1.0.14-dev/lib/pkgconfig:/nix/store/776xijk8rsb1b4c0dsxwq0k82bvm7mm9-libXpm-3.5.15-dev/lib/pkgconfig:/nix/store/qizdmm43xi65mdngal8bpbpqcdc8290d-libjpeg-turbo-2.1.5.1-dev/lib/pkgconfig:/nix/store/db7ix62fx4nvr9j1fjdvnznl2npff4pr-librsvg-2.55.1-dev/lib/pkgconfig:/nix/store/q0hg0951w1dv9y40m9ggln8phwil6lxc-gdk-pixbuf-2.42.10-dev/lib/pkgconfig:/nix/store/34rr5nvgljsc4bi3mxjxg8abmjr1f7hn-libtiff-4.5.0-dev/lib/pkgconfig:/nix/store/zwkr4kjcjs213pw9mhzi46bzlw6qwxzq-libdeflate-1.18/lib/pkgconfig:/nix/store/6na552yzwml88j8g5vqf5h9ir3vw8myi-xz-5.4.3-dev/lib/pkgconfig CXX=g++' Configured features: CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XAW3D XDBE XIM XPM LUCID ZLIB Important settings: value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: Diff Minor modes in effect: whitespace-mode: t envrc-global-mode: t envrc-mode: t global-git-commit-mode: t magit-auto-revert-mode: t shell-dirtrack-mode: t server-mode: t windmove-mode: t pixel-scroll-precision-mode: t savehist-mode: t save-place-mode: t tooltip-mode: t global-eldoc-mode: t show-paren-mode: t electric-indent-mode: t mouse-wheel-mode: t tab-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t context-menu-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t buffer-read-only: t line-number-mode: t indent-tabs-mode: t transient-mark-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t Load-path shadows: /home/sbaugh/.emacs.d/elpa/transient-0.3.7/transient hides /home/sbaugh/src/emacs/emacs-29/lisp/transient Features: (shadow sort emacsbug mpv tq org-timer org-clock org org-macro org-pcomplete org-list org-footnote org-faces org-entities ob-python python compat ob ob-tangle org-src ob-ref ob-lob ob-table ob-exp ob-comint ob-emacs-lisp ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys oc org-loaddefs cal-menu calendar cal-loaddefs org-version org-compat org-macs vc-src vc-sccs vc-svn vc-cvs vc-rcs gud noutline outline debug backtrace shortdoc pcmpl-unix pcmpl-gnu misc pulse color compile etags fileloop generator xref cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs bug-reference find-func cl-print help-fns radix-tree mail-extr rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap sgml-mode facemenu dom nxml-util nxml-enc xmltok whitespace log-view vc conf-mode dabbrev tabify man mule-util jka-compr tramp-cmds misearch multi-isearch vc-hg vc-bzr nix-mode nix-repl nix-shell nix-store nix-log nix-instantiate nix-shebang nix-format nix tramp-cache time-stamp tramp-sh tramp tramp-loaddefs trampver tramp-integration tramp-compat parse-time iso8601 cus-start vc-git vc-dispatcher dired-aux dired-x ffap sh-script smie treesit executable project files-x face-remap exwm-randr xcb-randr exwm exwm-input xcb-keysyms xcb-xkb exwm-manage exwm-floating xcb-cursor xcb-render exwm-layout exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types xcb-debug lui-autopaste circe advice lui-irc-colors irc gnutls lcs lui-logging lui-format lui tracking shorten thingatpt flyspell ispell circe-compat agda2 envrc inheritenv page-ext magit-extras magit-submodule magit-obsolete magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit magit-sequence magit-notes magit-worktree magit-tag magit-merge magit-branch magit-reset magit-files magit-refs magit-status magit magit-repos magit-apply magit-wip magit-log which-func imenu magit-diff smerge-mode diff diff-mode git-commit rx log-edit message sendmail yank-media puny dired desktop frameset dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util text-property-search time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log magit-core magit-autorevert autorevert filenotify magit-margin magit-transient magit-process with-editor shell pcomplete comint ansi-osc server ansi-color magit-mode transient cl-extra edmacro kmacro help-mode format-spec magit-git magit-section magit-utils crm dash windmove easy-mmode pixel-scroll cua-base ring modus-vivendi-theme modus-themes pcase cus-edit pp cus-load icons wid-edit savehist saveplace finder-inf ace-window-autoloads auctex-autoloads tex-site avy-autoloads circe-autoloads corfu-autoloads compat-autoloads csv-mode-autoloads cyberpunk-theme-autoloads debbugs-autoloads eat-autoloads envrc-autoloads exwm-autoloads ggtags-autoloads graphviz-dot-mode-autoloads htmlize-autoloads inheritenv-autoloads magit-autoloads git-commit-autoloads markdown-mode-autoloads mastodon-autoloads mentor-autoloads async-autoloads mpv-autoloads nix-mode-autoloads magit-section-autoloads dash-autoloads notmuch-autoloads persist-autoloads request-autoloads rust-mode-autoloads transient-autoloads url-scgi-autoloads vundo-autoloads which-key-autoloads info with-editor-autoloads xelb-autoloads xml-rpc-autoloads package browse-url url url-proxy url-privacy url-expand url-methods url-history url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic indonesian philippine cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget keymap hashtable-print-readable backquote threads dbusbind inotify dynamic-setting system-font-setting font-render-setting cairo x-toolkit x multi-tty make-network-process emacs) Memory information: ((conses 16 793112 58580) (symbols 48 42890 0) (strings 32 165901 6428) (string-bytes 1 8295556) (vectors 16 90751) (vector-slots 8 2007121 64111) (floats 8 418 578) (intervals 56 43463 2379) (buffers 984 111)) ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 2:24 bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping sbaugh @ 2023-09-13 2:30 ` sbaugh 2023-09-13 3:46 ` Jim Porter 2023-09-13 12:41 ` Eli Zaretskii 0 siblings, 2 replies; 59+ messages in thread From: sbaugh @ 2023-09-13 2:30 UTC (permalink / raw) To: 65902 [-- Attachment #1: Type: text/plain, Size: 413 bytes --] tags 65902 + patch quit This patch avoids the complicated scripting needed for emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, which mirrors emacs --funcall and allows emacsclient-mail.desktop to be basically the same as emacs-mail.desktop. I expect this to also be useful in other places; the need to escape arbitrary inputs before passing them to emacsclient is frequently annoying. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-funcall-to-emacsclient-to-avoid-escaping-argumen.patch --] [-- Type: text/x-patch, Size: 7904 bytes --] From 6c9dbda3aa5e3f1e3a5003bc0a15cf662b880d99 Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Tue, 12 Sep 2023 22:20:15 -0400 Subject: [PATCH] Add --funcall to emacsclient to avoid escaping arguments Passing arguments to functions through emacsclient --eval requires complicated escaping (as seen in emacsclient-mail.desktop before this change). The new --funcall argument behaves like emacs -f, and just passes the arguments as uninterpreted strings to the specified function. This simplifies use cases where arbitrary input needs to be passed to emacsclient. * etc/emacsclient-mail.desktop: Use --funcall. (bug#65902) * lib-src/emacsclient.c (longopts, decode_options, main): Add support for --funcall. * lisp/server.el (server-funcall-and-print): Add. (server-process-filter): Add support for -funcall and -funcallargs --- etc/emacsclient-mail.desktop | 7 ++----- lib-src/emacsclient.c | 36 ++++++++++++++++++++++++++++++++++-- lisp/server.el | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..fc6773a963b 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --funcall message-mailto %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,7 +13,7 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --funcall message-mailto %u [Desktop Action new-instance] Name=New Instance diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c index 698bf9b50ae..5bf6c05ef37 100644 --- a/lib-src/emacsclient.c +++ b/lib-src/emacsclient.c @@ -116,6 +116,9 @@ #define DEFAULT_TIMEOUT (30) /* True means args are expressions to be evaluated. --eval. */ static bool eval; +/* The function to call. Other arguments are passed as strings. --funcall. */ +static char *funcall; + /* True means open a new frame. --create-frame etc. */ static bool create_frame; @@ -169,6 +172,7 @@ #define DEFAULT_TIMEOUT (30) { "quiet", no_argument, NULL, 'q' }, { "suppress-output", no_argument, NULL, 'u' }, { "eval", no_argument, NULL, 'e' }, + { "funcall", required_argument, NULL, 'l' }, { "help", no_argument, NULL, 'H' }, { "version", no_argument, NULL, 'V' }, { "tty", no_argument, NULL, 't' }, @@ -552,6 +556,10 @@ decode_options (int argc, char **argv) eval = true; break; + case 'l': + funcall = optarg; + break; + case 'q': quiet = true; break; @@ -690,6 +698,7 @@ print_help_and_exit (void) -F ALIST, --frame-parameters=ALIST\n\ Set the parameters of a new frame\n\ -e, --eval Evaluate the FILE arguments as ELisp expressions\n\ +-l, --funcall FUNC Call ELisp FUNC, passing FILE arguments as strings\n\ -n, --no-wait Don't wait for the server to return\n\ -w, --timeout=SECONDS Seconds to wait before timing out\n\ -q, --quiet Don't display messages on success\n\ @@ -1953,7 +1962,7 @@ main (int argc, char **argv) /* Process options. */ decode_options (argc, argv); - if (! (optind < argc || eval || create_frame)) + if (! (optind < argc || eval || funcall || create_frame)) { message (true, ("%s: file name or argument required\n" "Try '%s --help' for more information\n"), @@ -1961,6 +1970,14 @@ main (int argc, char **argv) exit (EXIT_FAILURE); } + if (eval && funcall) + { + message (true, ("%s: can't pass both --eval and --funcall\n" + "Try '%s --help' for more information\n"), + progname, progname); + exit (EXIT_FAILURE); + } + #ifdef SOCKETS_IN_FILE_SYSTEM if (tty) { @@ -2080,6 +2097,13 @@ main (int argc, char **argv) send_to_emacs (emacs_socket, " "); continue; } + else if (funcall) + { + send_to_emacs (emacs_socket, "-funcallarg "); + quote_argument (emacs_socket, argv[i]); + send_to_emacs (emacs_socket, " "); + continue; + } char *p = argv[i]; if (*p == '+') @@ -2136,10 +2160,18 @@ main (int argc, char **argv) send_to_emacs (emacs_socket, " "); } + if (funcall) + { + send_to_emacs (emacs_socket, "-funcall "); + quote_argument (emacs_socket, funcall); + send_to_emacs (emacs_socket, " "); + } + + send_to_emacs (emacs_socket, "\n"); /* Wait for an answer. */ - if (!eval && !tty && !nowait && !quiet && 0 <= process_grouping ()) + if (!eval && !funcall && !tty && !nowait && !quiet && 0 <= process_grouping ()) { printf ("Waiting for Emacs..."); skiplf = false; diff --git a/lisp/server.el b/lisp/server.el index c3325e5a24c..da3319581ea 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -873,6 +873,17 @@ server-eval-and-print (point-min) (point-max)))) (server-reply-print (server-quote-arg text) proc))))))) +(defun server-funcall-and-print (func args proc) + "Call FUNC on ARGS and send the result back to client PROC." + (let ((v (with-local-quit (eval (apply (intern func) args) t)))) + (when proc + (with-temp-buffer + (let ((standard-output (current-buffer))) + (pp v) + (let ((text (buffer-substring-no-properties + (point-min) (point-max)))) + (server-reply-print (server-quote-arg text) proc))))))) + (defconst server-msg-size 1024 "Maximum size of a message sent to a client.") @@ -1196,6 +1207,7 @@ server-process-filter tty-type ; string. files filepos + funcallargs args-left) ;; Remove this line from STRING. (setq string (substring string (match-end 0))) @@ -1323,6 +1335,28 @@ server-process-filter commands) (setq filepos nil))) + ;; -funcall FUNC: Call a function on arguments. + ("-funcall" + (if use-current-frame + (setq use-current-frame 'always)) + (let ((func (pop args-left))) + (if coding-system + (setq func (decode-coding-string func coding-system))) + (push (lambda () (server-funcall-and-print func funcallargs proc)) + commands) + (setq funcallargs nil) + (setq filepos nil))) + + ;; -funcallarg ARG: Add an argument for later -funcall. + ("-funcallarg" + (if use-current-frame + (setq use-current-frame 'always)) + (let ((arg (pop args-left))) + (if coding-system + (setq arg (decode-coding-string arg coding-system))) + (push arg funcallargs) + (setq filepos nil))) + ;; -env NAME=VALUE: An environment variable. ("-env" (let ((var (pop args-left))) -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 2:30 ` sbaugh @ 2023-09-13 3:46 ` Jim Porter 2023-09-13 8:00 ` Robert Pluim ` (2 more replies) 2023-09-13 12:41 ` Eli Zaretskii 1 sibling, 3 replies; 59+ messages in thread From: Jim Porter @ 2023-09-13 3:46 UTC (permalink / raw) To: sbaugh, 65902 On 9/12/2023 7:30 PM, sbaugh@catern.com wrote: > tags 65902 + patch > quit > > This patch avoids the complicated scripting needed for > emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, > which mirrors emacs --funcall and allows emacsclient-mail.desktop to be > basically the same as emacs-mail.desktop. I think this is actually the same as the (very long) bug#57752, so thanks for working on this. (It was on my list of things to get to, but I just haven't had time.) Over there, we agreed that something like your patch is wanted, albeit with two caveats: 1. Since "--funcall" for the regular "emacs" binary doesn't pass arguments to the function, how about we call this option "--apply" instead? 2. It would be great if we could get "--apply" for the regular "emacs" binary too, so that both programs work the same way (at least in this regard). Even better, if you could forward "--apply" from "emacsclient" to the alternate editor (which would be "emacs" 99% of the time) automatically. That works, in a roundabout way, for the Emacs daemon, but not if the alternate editor is "emacs". ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 3:46 ` Jim Porter @ 2023-09-13 8:00 ` Robert Pluim 2023-09-13 13:06 ` Eli Zaretskii 2023-09-13 12:41 ` Eli Zaretskii 2023-09-13 12:57 ` sbaugh 2 siblings, 1 reply; 59+ messages in thread From: Robert Pluim @ 2023-09-13 8:00 UTC (permalink / raw) To: Jim Porter; +Cc: sbaugh, 65902 >>>>> On Tue, 12 Sep 2023 20:46:54 -0700, Jim Porter <jporterbugs@gmail.com> said: Jim> On 9/12/2023 7:30 PM, sbaugh@catern.com wrote: >> tags 65902 + patch >> quit >> This patch avoids the complicated scripting needed for >> emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, >> which mirrors emacs --funcall and allows emacsclient-mail.desktop to be >> basically the same as emacs-mail.desktop. Jim> I think this is actually the same as the (very long) bug#57752, so Jim> thanks for working on this. (It was on my list of things to get to, Jim> but I just haven't had time.) 57752 got bogged down in design paralysis :-) The idea looks good to me. Jim> Over there, we agreed that something like your patch is wanted, albeit Jim> with two caveats: Jim> 1. Since "--funcall" for the regular "emacs" binary doesn't pass Jim> arguments to the function, how about we call this option "--apply" Jim> instead? Yes, that sounds right. Jim> 2. It would be great if we could get "--apply" for the regular "emacs" Jim> binary too, so that both programs work the same way (at least in this Jim> regard). Even better, if you could forward "--apply" from Jim> "emacsclient" to the alternate editor (which would be "emacs" 99% of Jim> the time) automatically. That works, in a roundabout way, for the Jim> Emacs daemon, but not if the alternate editor is "emacs". Consistency would be good here. Robert -- ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 8:00 ` Robert Pluim @ 2023-09-13 13:06 ` Eli Zaretskii 2023-09-13 14:22 ` Robert Pluim 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-13 13:06 UTC (permalink / raw) To: Robert Pluim; +Cc: 65902, jporterbugs, sbaugh > Cc: sbaugh@catern.com, 65902@debbugs.gnu.org > From: Robert Pluim <rpluim@gmail.com> > Date: Wed, 13 Sep 2023 10:00:00 +0200 > > >>>>> On Tue, 12 Sep 2023 20:46:54 -0700, Jim Porter <jporterbugs@gmail.com> said: > > Jim> On 9/12/2023 7:30 PM, sbaugh@catern.com wrote: > >> tags 65902 + patch > >> quit > >> This patch avoids the complicated scripting needed for > >> emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, > >> which mirrors emacs --funcall and allows emacsclient-mail.desktop to be > >> basically the same as emacs-mail.desktop. > > Jim> I think this is actually the same as the (very long) bug#57752, so > Jim> thanks for working on this. (It was on my list of things to get to, > Jim> but I just haven't had time.) > > 57752 got bogged down in design paralysis :-) And where do you think we will get here, if we retrace all the arguments and counter-arguments voiced there? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 13:06 ` Eli Zaretskii @ 2023-09-13 14:22 ` Robert Pluim 0 siblings, 0 replies; 59+ messages in thread From: Robert Pluim @ 2023-09-13 14:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, jporterbugs, sbaugh >>>>> On Wed, 13 Sep 2023 16:06:07 +0300, Eli Zaretskii <eliz@gnu.org> said: >> Cc: sbaugh@catern.com, 65902@debbugs.gnu.org >> From: Robert Pluim <rpluim@gmail.com> >> Date: Wed, 13 Sep 2023 10:00:00 +0200 >> >> >>>>> On Tue, 12 Sep 2023 20:46:54 -0700, Jim Porter <jporterbugs@gmail.com> said: >> Jim> On 9/12/2023 7:30 PM, sbaugh@catern.com wrote: >> >> tags 65902 + patch >> >> quit >> >> This patch avoids the complicated scripting needed for >> >> emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, >> >> which mirrors emacs --funcall and allows emacsclient-mail.desktop to be >> >> basically the same as emacs-mail.desktop. >> Jim> I think this is actually the same as the (very long) bug#57752, so Jim> thanks for working on this. (It was on my list of things to get to, Jim> but I just haven't had time.) >> >> 57752 got bogged down in design paralysis :-) Eli> And where do you think we will get here, if we retrace all the Eli> arguments and counter-arguments voiced there? I was not intending to do that, I was intending to advocate for Spencerʼs work to go in pretty much as-is (modulo calling it '--apply' rather than '--funcall'). Robert -- ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 3:46 ` Jim Porter 2023-09-13 8:00 ` Robert Pluim @ 2023-09-13 12:41 ` Eli Zaretskii 2023-09-13 12:57 ` sbaugh 2 siblings, 0 replies; 59+ messages in thread From: Eli Zaretskii @ 2023-09-13 12:41 UTC (permalink / raw) To: Jim Porter; +Cc: sbaugh, 65902 > Date: Tue, 12 Sep 2023 20:46:54 -0700 > From: Jim Porter <jporterbugs@gmail.com> > > On 9/12/2023 7:30 PM, sbaugh@catern.com wrote: > > tags 65902 + patch > > quit > > > > This patch avoids the complicated scripting needed for > > emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, > > which mirrors emacs --funcall and allows emacsclient-mail.desktop to be > > basically the same as emacs-mail.desktop. > > I think this is actually the same as the (very long) bug#57752, so > thanks for working on this. (It was on my list of things to get to, but > I just haven't had time.) > > Over there, we agreed that something like your patch is wanted, albeit > with two caveats: > > 1. Since "--funcall" for the regular "emacs" binary doesn't pass > arguments to the function, how about we call this option "--apply" instead? > > 2. It would be great if we could get "--apply" for the regular "emacs" > binary too, so that both programs work the same way (at least in this > regard). Even better, if you could forward "--apply" from "emacsclient" > to the alternate editor (which would be "emacs" 99% of the time) > automatically. That works, in a roundabout way, for the Emacs daemon, > but not if the alternate editor is "emacs". This is how that bug's discussion started. Are we going to have it all one more time? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 3:46 ` Jim Porter 2023-09-13 8:00 ` Robert Pluim 2023-09-13 12:41 ` Eli Zaretskii @ 2023-09-13 12:57 ` sbaugh 2 siblings, 0 replies; 59+ messages in thread From: sbaugh @ 2023-09-13 12:57 UTC (permalink / raw) To: Jim Porter; +Cc: 65902 [-- Attachment #1: Type: text/plain, Size: 1750 bytes --] Jim Porter <jporterbugs@gmail.com> writes: > On 9/12/2023 7:30 PM, sbaugh@catern.com wrote: >> tags 65902 + patch >> quit >> This patch avoids the complicated scripting needed for >> emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, >> which mirrors emacs --funcall and allows emacsclient-mail.desktop to be >> basically the same as emacs-mail.desktop. > > I think this is actually the same as the (very long) bug#57752, so > thanks for working on this. (It was on my list of things to get to, > but I just haven't had time.) > > Over there, we agreed that something like your patch is wanted, albeit > with two caveats: > > 1. Since "--funcall" for the regular "emacs" binary doesn't pass > arguments to the function, how about we call this option "--apply" > instead? Ah, for some reason I thought --funcall passed arguments. Done in attached patch. > 2. It would be great if we could get "--apply" for the regular "emacs" > binary too, so that both programs work the same way (at least in this > regard). Done. Although the behavior is slightly different: emacs --apply calls the function with subsequent FILE arguments, and emacsclient --apply calls the function with all FILE arguments. The "subsequent FILE arguments" behavior is probably better, but I don't know a way to do that with getopt (which emacsclient uses). > Even better, if you could forward "--apply" from > "emacsclient" to the alternate editor (which would be "emacs" 99% of > the time) automatically. That works, in a roundabout way, for the > Emacs daemon, but not if the alternate editor is "emacs". This happens automatically anyway: When emacsclient starts the daemon, it sends the --apply request. I think that's exactly what you'd want. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-apply-argument-to-avoid-escaping-arguments.patch --] [-- Type: text/x-patch, Size: 12161 bytes --] From 4881017055ea6831ee7fe2d722eb79856946d907 Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Tue, 12 Sep 2023 22:20:15 -0400 Subject: [PATCH] Add --apply argument to avoid escaping arguments Passing arguments to functions through emacs --eval or emacsclient --eval requires complicated escaping (as seen in emacsclient-mail.desktop before this change). The new --apply argument for both emacs and emacsclient passes command line arguments as uninterpreted strings to the specified function. This simplifies use cases where arbitrary input needs to be passed to Emacs. Note that there's a minor difference in behavior between emacs --apply and emacsclient --apply: emacs --apply func calls func with only command line arguments occuring after --apply, emacsclient --apply func calls func with all command line arguments, even those which occurred before --apply. This is hard to avoid since emacsclient uses getopt instead of fancier custom Lisp argument parsing in Emacs. * etc/emacsclient-mail.desktop: Use --apply. (bug#65902) * lib-src/emacsclient.c (longopts, decode_options, main): Add support for --apply. * lisp/server.el (server-apply-and-print): Add. (server-process-filter): Add support for -apply and -applyargs * lisp/startup.el (command-line-1): Add support for --apply. * src/emacs.c (usage_message, standard_args): Add support for --apply. --- etc/emacsclient-mail.desktop | 9 +++------ lib-src/emacsclient.c | 36 ++++++++++++++++++++++++++++++++++-- lisp/server.el | 34 ++++++++++++++++++++++++++++++++++ lisp/startup.el | 18 +++++++++++++++--- src/emacs.c | 3 +++ 5 files changed, 89 insertions(+), 11 deletions(-) diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..750fcddacdc 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --apply message-mailto -- %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,8 +13,8 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --apply message-mailto -- %u [Desktop Action new-instance] Name=New Instance -Exec=emacs -f message-mailto %u +Exec=emacs --apply message-mailto -- %u diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c index 698bf9b50ae..159c22d1ae9 100644 --- a/lib-src/emacsclient.c +++ b/lib-src/emacsclient.c @@ -116,6 +116,9 @@ #define DEFAULT_TIMEOUT (30) /* True means args are expressions to be evaluated. --eval. */ static bool eval; +/* The function to call. Other arguments are passed as strings. --apply. */ +static char *apply; + /* True means open a new frame. --create-frame etc. */ static bool create_frame; @@ -169,6 +172,7 @@ #define DEFAULT_TIMEOUT (30) { "quiet", no_argument, NULL, 'q' }, { "suppress-output", no_argument, NULL, 'u' }, { "eval", no_argument, NULL, 'e' }, + { "apply", required_argument, NULL, 'y' }, { "help", no_argument, NULL, 'H' }, { "version", no_argument, NULL, 'V' }, { "tty", no_argument, NULL, 't' }, @@ -552,6 +556,10 @@ decode_options (int argc, char **argv) eval = true; break; + case 'y': + apply = optarg; + break; + case 'q': quiet = true; break; @@ -690,6 +698,7 @@ print_help_and_exit (void) -F ALIST, --frame-parameters=ALIST\n\ Set the parameters of a new frame\n\ -e, --eval Evaluate the FILE arguments as ELisp expressions\n\ +-y, --apply FUNC Call ELisp FUNC, passing all FILE arguments as strings\n\ -n, --no-wait Don't wait for the server to return\n\ -w, --timeout=SECONDS Seconds to wait before timing out\n\ -q, --quiet Don't display messages on success\n\ @@ -1953,7 +1962,7 @@ main (int argc, char **argv) /* Process options. */ decode_options (argc, argv); - if (! (optind < argc || eval || create_frame)) + if (! (optind < argc || eval || apply || create_frame)) { message (true, ("%s: file name or argument required\n" "Try '%s --help' for more information\n"), @@ -1961,6 +1970,14 @@ main (int argc, char **argv) exit (EXIT_FAILURE); } + if (eval && apply) + { + message (true, ("%s: can't pass both --eval and --apply\n" + "Try '%s --help' for more information\n"), + progname, progname); + exit (EXIT_FAILURE); + } + #ifdef SOCKETS_IN_FILE_SYSTEM if (tty) { @@ -2080,6 +2097,13 @@ main (int argc, char **argv) send_to_emacs (emacs_socket, " "); continue; } + else if (apply) + { + send_to_emacs (emacs_socket, "-applyarg "); + quote_argument (emacs_socket, argv[i]); + send_to_emacs (emacs_socket, " "); + continue; + } char *p = argv[i]; if (*p == '+') @@ -2136,10 +2160,18 @@ main (int argc, char **argv) send_to_emacs (emacs_socket, " "); } + if (apply) + { + send_to_emacs (emacs_socket, "-apply "); + quote_argument (emacs_socket, apply); + send_to_emacs (emacs_socket, " "); + } + + send_to_emacs (emacs_socket, "\n"); /* Wait for an answer. */ - if (!eval && !tty && !nowait && !quiet && 0 <= process_grouping ()) + if (!eval && !apply && !tty && !nowait && !quiet && 0 <= process_grouping ()) { printf ("Waiting for Emacs..."); skiplf = false; diff --git a/lisp/server.el b/lisp/server.el index c3325e5a24c..5981e90625d 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -873,6 +873,17 @@ server-eval-and-print (point-min) (point-max)))) (server-reply-print (server-quote-arg text) proc))))))) +(defun server-apply-and-print (func args proc) + "Call FUNC on ARGS and send the result back to client PROC." + (let ((v (with-local-quit (eval (apply (intern func) args) t)))) + (when proc + (with-temp-buffer + (let ((standard-output (current-buffer))) + (pp v) + (let ((text (buffer-substring-no-properties + (point-min) (point-max)))) + (server-reply-print (server-quote-arg text) proc))))))) + (defconst server-msg-size 1024 "Maximum size of a message sent to a client.") @@ -1196,6 +1207,7 @@ server-process-filter tty-type ; string. files filepos + applyargs args-left) ;; Remove this line from STRING. (setq string (substring string (match-end 0))) @@ -1323,6 +1335,28 @@ server-process-filter commands) (setq filepos nil))) + ;; -apply FUNC: Call a function on arguments. + ("-apply" + (if use-current-frame + (setq use-current-frame 'always)) + (let ((func (pop args-left))) + (if coding-system + (setq func (decode-coding-string func coding-system))) + (push (lambda () (server-apply-and-print func applyargs proc)) + commands) + (setq applyargs nil) + (setq filepos nil))) + + ;; -applyarg ARG: Add an argument for later -apply. + ("-applyarg" + (if use-current-frame + (setq use-current-frame 'always)) + (let ((arg (pop args-left))) + (if coding-system + (setq arg (decode-coding-string arg coding-system))) + (push arg applyargs) + (setq filepos nil))) + ;; -env NAME=VALUE: An environment variable. ("-env" (let ((var (pop args-left))) diff --git a/lisp/startup.el b/lisp/startup.el index 7f601668369..bb8da76bdf1 100644 --- a/lisp/startup.el +++ b/lisp/startup.el @@ -2531,10 +2531,11 @@ command-line-1 ;; straight away upon any --directory/-L option. splice just-files ;; t if this follows the magic -- option. + applysym applyargs ;; function and arguments for --apply ;; This includes our standard options' long versions ;; and long versions of what's on command-switch-alist. (longopts - (append '("--funcall" "--load" "--insert" "--kill" + (append '("--funcall" "--apply" "--load" "--insert" "--kill" "--dump-file" "--seccomp" "--directory" "--eval" "--execute" "--no-splash" "--find-file" "--visit" "--file" "--no-desktop") @@ -2632,6 +2633,11 @@ command-line-1 (command-execute tem) (funcall tem))) + ((member argi '("-y" "-apply")) + (setq inhibit-startup-screen t) + ;; Subsequent file args will be accumulated into applyargs. + (setq applysym (intern (or argval (pop command-line-args-left))))) + ((member argi '("-eval" "-execute")) (setq inhibit-startup-screen t) (let* ((str-expr (or argval (pop command-line-args-left))) @@ -2763,13 +2769,19 @@ command-line-1 ;; screen for -nw? (unless initial-window-system (setq inhibit-startup-screen t)) - (funcall process-file-arg orig-argi))))) + (if applysym + (push orig-argi applyargs) + (funcall process-file-arg orig-argi)))))) ;; In unusual circumstances, the execution of Lisp code due ;; to command-line options can cause the last visible frame ;; to be deleted. In this case, kill emacs to avoid an ;; abort later. - (unless (frame-live-p (selected-frame)) (kill-emacs nil))))))) + (unless (frame-live-p (selected-frame)) (kill-emacs nil)))) + + ;; Call the function specified with --apply, if any. + (when applysym + (apply applysym (nreverse applyargs)))))) (when (eq initial-buffer-choice t) ;; When `initial-buffer-choice' equals t make sure that *scratch* diff --git a/src/emacs.c b/src/emacs.c index 80a013b68df..8de40936250 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -323,6 +323,8 @@ #define MAIN_PROGRAM --file FILE visit FILE\n\ --find-file FILE visit FILE\n\ --funcall, -f FUNC call Emacs Lisp function FUNC with no arguments\n\ +--apply, -y FUNC call Emacs Lisp function FUNC, passing\n\ + subsequent positional FILE arguments as strings\n\ --insert FILE insert contents of FILE into current buffer\n\ --kill exit without asking for confirmation\n\ --load, -l FILE load Emacs Lisp FILE using the load function\n\ @@ -2648,6 +2650,7 @@ main (int argc, char **argv) option. In any case, this is entirely an internal option. */ { "-scriptload", NULL, 0, 1 }, { "-f", "--funcall", 0, 1 }, + { "-y", "--apply", 0, 1 }, { "-funcall", 0, 0, 1 }, { "-eval", "--eval", 0, 1 }, { "-execute", "--execute", 0, 1 }, -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 2:30 ` sbaugh 2023-09-13 3:46 ` Jim Porter @ 2023-09-13 12:41 ` Eli Zaretskii 2023-09-13 13:01 ` sbaugh 1 sibling, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-13 12:41 UTC (permalink / raw) To: sbaugh; +Cc: 65902 > From: sbaugh@catern.com > Date: Wed, 13 Sep 2023 02:30:08 +0000 (UTC) > > This patch avoids the complicated scripting needed for > emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, > which mirrors emacs --funcall and allows emacsclient-mail.desktop to be > basically the same as emacs-mail.desktop. > > I expect this to also be useful in other places; the need to escape > arbitrary inputs before passing them to emacsclient is frequently > annoying. Is quoting the only issue with --eval? If so, why not have a variant of --eval that quotes the argument by itself, like you do in the patch, i.e. by using quote_argument, instead of inventing yet another weird option with its own small DSL and extra-special rules of how to write the command line with it? More generally, I cannot say I'm happy that we basically are reiterating everything that was said in bug#57752 instead of picking up where it left off. Why is it useful to have another discussion like that one (which will probably end up at the same impasse)? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 12:41 ` Eli Zaretskii @ 2023-09-13 13:01 ` sbaugh 2023-09-13 13:26 ` Eli Zaretskii 0 siblings, 1 reply; 59+ messages in thread From: sbaugh @ 2023-09-13 13:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902 Eli Zaretskii <eliz@gnu.org> writes: >> From: sbaugh@catern.com >> Date: Wed, 13 Sep 2023 02:30:08 +0000 (UTC) >> >> This patch avoids the complicated scripting needed for >> emacsclient-mail.desktop by adding a new flag to emacsclient, --funcall, >> which mirrors emacs --funcall and allows emacsclient-mail.desktop to be >> basically the same as emacs-mail.desktop. >> >> I expect this to also be useful in other places; the need to escape >> arbitrary inputs before passing them to emacsclient is frequently >> annoying. > > Is quoting the only issue with --eval? If so, why not have a variant > of --eval that quotes the argument by itself, like you do in the > patch, i.e. by using quote_argument, instead of inventing yet another > weird option with its own small DSL and extra-special rules of how to > write the command line with it? I am not sure what you're suggesting. Can you show how the equivalent of: emacsclient --apply message-mailto -- %u would work with that design? > More generally, I cannot say I'm happy that we basically are > reiterating everything that was said in bug#57752 instead of picking > up where it left off. Why is it useful to have another discussion > like that one (which will probably end up at the same impasse)? I admit I didn't see that bug until now, but the way in which this is different is that I have now read the discussion, incorporated all the advice, and I'm actually implementing it :) Probably these bugs could/should be merged though. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 13:01 ` sbaugh @ 2023-09-13 13:26 ` Eli Zaretskii 2023-09-16 13:30 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-13 13:26 UTC (permalink / raw) To: sbaugh; +Cc: 65902 > From: sbaugh@catern.com > Date: Wed, 13 Sep 2023 13:01:52 +0000 (UTC) > Cc: 65902@debbugs.gnu.org > > Eli Zaretskii <eliz@gnu.org> writes: > > > Is quoting the only issue with --eval? If so, why not have a variant > > of --eval that quotes the argument by itself, like you do in the > > patch, i.e. by using quote_argument, instead of inventing yet another > > weird option with its own small DSL and extra-special rules of how to > > write the command line with it? > > I am not sure what you're suggesting. Can you show how the equivalent > of: > > emacsclient --apply message-mailto -- %u > > would work with that design? emacsclient --qeval '(message-mailto %u)' > > More generally, I cannot say I'm happy that we basically are > > reiterating everything that was said in bug#57752 instead of picking > > up where it left off. Why is it useful to have another discussion > > like that one (which will probably end up at the same impasse)? > > I admit I didn't see that bug until now, but the way in which this is > different is that I have now read the discussion, incorporated all the > advice, and I'm actually implementing it :) You are well under way to reach the same final point we ended up there, because both the arguments people voice here and your responses/reactions are the same as we saw there. For example, we already have an additional option to Emacs, although the original problem had nothing to do with Emacs itself. Let's not do that this time, okay? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 13:26 ` Eli Zaretskii @ 2023-09-16 13:30 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 59+ messages in thread From: Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-09-16 13:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: sbaugh, 65902 Eli Zaretskii <eliz@gnu.org> writes: >> From: sbaugh@catern.com >> Date: Wed, 13 Sep 2023 13:01:52 +0000 (UTC) >> Cc: 65902@debbugs.gnu.org >> >> Eli Zaretskii <eliz@gnu.org> writes: >> >> > Is quoting the only issue with --eval? If so, why not have a variant >> > of --eval that quotes the argument by itself, like you do in the >> > patch, i.e. by using quote_argument, instead of inventing yet another >> > weird option with its own small DSL and extra-special rules of how to >> > write the command line with it? >> >> I am not sure what you're suggesting. Can you show how the equivalent >> of: >> >> emacsclient --apply message-mailto -- %u >> >> would work with that design? > > emacsclient --qeval '(message-mailto %u)' > Try desktop-file-validate. ' is a reserved character. I have a desktop file for a similar use case, I validated that desktop file using desktop-file-validate and got the following output: emacs-gnus.desktop: error: value "emacsclient --create-frame --eval '(gnus)'" for key "Exec" in group "Desktop Entry" contains a reserved character ''' outside of a quote ^ permalink raw reply [flat|nested] 59+ messages in thread
[parent not found: <fe2cc764-86c6-4840-80b7-8f3a3778b374@email.android.com>]
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping [not found] <fe2cc764-86c6-4840-80b7-8f3a3778b374@email.android.com> @ 2023-09-13 14:50 ` Eli Zaretskii 2023-09-13 15:01 ` Andreas Schwab 2023-09-13 15:23 ` Spencer Baugh 0 siblings, 2 replies; 59+ messages in thread From: Eli Zaretskii @ 2023-09-13 14:50 UTC (permalink / raw) To: Spencer Baugh; +Cc: 65902 > Date: Wed, 13 Sep 2023 14:08:01 +0000 (UTC) > From: Spencer Baugh <sbaugh@catern.com> > Cc: 65902@debbugs.gnu.org > > On Sep 13, 2023 09:26, Eli Zaretskii <eliz@gnu.org> wrote: > > > I am not sure what you're suggesting. Can you show how the equivalent > > of: > > > > emacsclient --apply message-mailto -- %u > > > > would work with that design? > > emacsclient --qeval '(message-mailto %u)' > > I don't think this can work in general for arbitrary user input: what if %u is replaced with something > that contains parentheses? They are inside '..', so the only one who'd care is Emacs, not the shell. In which case it's the job of whoever provides the value for %u to handle that. And anyway, how is that different from the same problem happening with your suggested --funcall or --apply? they will bump into the same issues. > Let's not do that this time, okay? > > Agreed, I think we reached a consensus in that bug and now I am implementing that consensus. AFAIU, there was no consensus reached there, so I'm unsure what are you alluding to here. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 14:50 ` Eli Zaretskii @ 2023-09-13 15:01 ` Andreas Schwab 2023-09-13 15:23 ` Spencer Baugh 1 sibling, 0 replies; 59+ messages in thread From: Andreas Schwab @ 2023-09-13 15:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Spencer Baugh, 65902 On Sep 13 2023, Eli Zaretskii wrote: > They are inside '..', so the only one who'd care is Emacs, not the > shell. In which case it's the job of whoever provides the value for > %u to handle that. The spec does not allow that. The %u marker cannot be part of a quoted argument. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 14:50 ` Eli Zaretskii 2023-09-13 15:01 ` Andreas Schwab @ 2023-09-13 15:23 ` Spencer Baugh 2023-09-13 16:19 ` Jim Porter 2023-09-13 19:13 ` Eli Zaretskii 1 sibling, 2 replies; 59+ messages in thread From: Spencer Baugh @ 2023-09-13 15:23 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Spencer Baugh, 65902 Eli Zaretskii <eliz@gnu.org> writes: >> Date: Wed, 13 Sep 2023 14:08:01 +0000 (UTC) >> From: Spencer Baugh <sbaugh@catern.com> >> Cc: 65902@debbugs.gnu.org >> >> On Sep 13, 2023 09:26, Eli Zaretskii <eliz@gnu.org> wrote: >> >> > I am not sure what you're suggesting. Can you show how the equivalent >> > of: >> > >> > emacsclient --apply message-mailto -- %u >> > >> > would work with that design? >> >> emacsclient --qeval '(message-mailto %u)' >> >> I don't think this can work in general for arbitrary user input: what if %u is replaced with something >> that contains parentheses? > > They are inside '..', so the only one who'd care is Emacs, not the > shell. Agreed. The problem I'm referring to is in Emacs, interpreting arbitrary input from the web as code. (The .desktop commands don't even use a shell, a shell doesn't need to be involved at any point) > In which case it's the job of whoever provides the value for > %u to handle that. The value for %u is an arbitrary string from some other application which wants to open a mailto: URI, and passes it to xdg-open which then passes it to Emacs. Other applications are not aware of what escaping is needed to make Emacs not interpret it as code. And indeed, there's no point in doing that: Emacs is in the best position to do that escaping, if it needs to be done. > And anyway, how is that different from the same problem happening with > your suggested --funcall or --apply? they will bump into the same > issues. No, they won't: --apply passes the arguments as a string, without ever trying to parse them as Lisp. Let's be concrete: imagine %u is replaced with (shell-command "rm -r /") as could happen if an application receives some malicious input. The command line with --qeval is: emacsclient --qeval '(message-mailto (shell-command "rm -r /"))' Emacs receives -eval (message-mailto (shell-command "rm -r /")) and evals (message-mailto (shell-command "rm -r /")) and deletes your files. The command line with --apply is: emacsclient --apply message-mailto '(shell-command "rm -r /")' Emacs receives -apply message-mailto --applyarg (shell-command "rm -r /") and evals (message-mailto "(shell-command \"rm -r /\")") and nothing bad happens. Emacs just needs to get the verbatim string without trying to parse it as Lisp at any point. This is an extremely standard security technique when dealing with malicious input, which is why the previous thread converged on it so quickly. >> Let's not do that this time, okay? >> >> Agreed, I think we reached a consensus in that bug and now I am implementing that consensus. > > AFAIU, there was no consensus reached there, so I'm unsure what are > you alluding to here. Everyone in that thread agreed that something like this --apply design (which passes the strings verbatim to Emacs without evaling them) is what we need, they were just discussing the exact design, and in the end the design that everyone who posted agreed on, matched what I have implemented... I don't think we need to relitigate it. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 15:23 ` Spencer Baugh @ 2023-09-13 16:19 ` Jim Porter 2023-09-13 19:13 ` Eli Zaretskii 1 sibling, 0 replies; 59+ messages in thread From: Jim Porter @ 2023-09-13 16:19 UTC (permalink / raw) To: Spencer Baugh, Eli Zaretskii; +Cc: Spencer Baugh, 65902 On 9/13/2023 8:23 AM, Spencer Baugh wrote: > Eli Zaretskii <eliz@gnu.org> writes: >> AFAIU, there was no consensus reached there, so I'm unsure what are >> you alluding to here. > > Everyone in that thread agreed that something like this --apply design > (which passes the strings verbatim to Emacs without evaling them) is > what we need, they were just discussing the exact design, and in the end > the design that everyone who posted agreed on, matched what I have > implemented... I don't think we need to relitigate it. That was my impression as well. Most of the discussion in that bug was among Robert (already commenting on this bug in agreement with the current patch), Gregory (who I believe supports the --apply design[1]), Lars (who proposed a form of the --apply design to handle multiple --apply calls[2]), and me (I support --apply, using Lars' semantics if we can get it). [1] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57752#119 [2] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57752#122 ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 15:23 ` Spencer Baugh 2023-09-13 16:19 ` Jim Porter @ 2023-09-13 19:13 ` Eli Zaretskii 2023-09-13 19:33 ` Jim Porter 1 sibling, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-13 19:13 UTC (permalink / raw) To: Spencer Baugh; +Cc: sbaugh, 65902 > From: Spencer Baugh <sbaugh@janestreet.com> > Cc: Spencer Baugh <sbaugh@catern.com>, 65902@debbugs.gnu.org > Date: Wed, 13 Sep 2023 11:23:06 -0400 > > >> Agreed, I think we reached a consensus in that bug and now I am implementing that consensus. > > > > AFAIU, there was no consensus reached there, so I'm unsure what are > > you alluding to here. > > Everyone in that thread agreed that something like this --apply design > (which passes the strings verbatim to Emacs without evaling them) is > what we need, they were just discussing the exact design, and in the end > the design that everyone who posted agreed on, matched what I have > implemented... That's not my reading of that long discussion. And I don't understand why we need to add any options to Emacs itself, btw. The suggestion to have some "symmetry" here was one of the reasons that discussion got nowhere. So let's learn from that mistake, at least. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 19:13 ` Eli Zaretskii @ 2023-09-13 19:33 ` Jim Porter 2023-09-13 20:00 ` Spencer Baugh 2023-09-14 5:10 ` Eli Zaretskii 0 siblings, 2 replies; 59+ messages in thread From: Jim Porter @ 2023-09-13 19:33 UTC (permalink / raw) To: Eli Zaretskii, Spencer Baugh; +Cc: sbaugh, 65902 On 9/13/2023 12:13 PM, Eli Zaretskii wrote: > And I don't understand why we need to add any options to Emacs itself, > btw. The suggestion to have some "symmetry" here was one of the > reasons that discussion got nowhere. So let's learn from that > mistake, at least. There's a practical benefit to this. If you have $EDITOR set in your environment to something like "emacsclient --alternate-editor=emacs", then it would be nice if you could say this: $EDITOR --apply some-func arg1 arg2 and have it do the same thing whether or not there was already an Emacs server running. The symmetry between the two commands (plus proper argument forwarding) would make that work. However, if people can't agree, then we could probably drop that part. To me, it sounds like people *do* agree that this would be good to have though. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 19:33 ` Jim Porter @ 2023-09-13 20:00 ` Spencer Baugh 2023-09-13 20:16 ` Jim Porter 2023-09-14 5:10 ` Eli Zaretskii 1 sibling, 1 reply; 59+ messages in thread From: Spencer Baugh @ 2023-09-13 20:00 UTC (permalink / raw) To: Jim Porter; +Cc: sbaugh, Eli Zaretskii, 65902 Jim Porter <jporterbugs@gmail.com> writes: > On 9/13/2023 12:13 PM, Eli Zaretskii wrote: >> And I don't understand why we need to add any options to Emacs itself, >> btw. The suggestion to have some "symmetry" here was one of the >> reasons that discussion got nowhere. So let's learn from that >> mistake, at least. > > There's a practical benefit to this. If you have $EDITOR set in your > environment to something like "emacsclient --alternate-editor=emacs", > then it would be nice if you could say this: > > $EDITOR --apply some-func arg1 arg2 > > and have it do the same thing whether or not there was already an > Emacs server running. The symmetry between the two commands (plus > proper argument forwarding) would make that work. This already works out of the box with --alternate-editor='', no need for any argument forwarding, and no need for support for this argument in Emacs itself. It doesn't work with --alternate-editor=emacs, but I think that is OK. I think the alternate-editor='' configuration is far more common, and that works just fine. And we have not before done any special casing for forwarding arguments from emacsclient to alternate-editor=emacs, so I don't think there's any reason to start now. (I personally think that alternate-editor=emacs is somewhat ill-advised, since it doesn't ensure that the server gets started) That being said, supporting --apply in Emacs itself is fine with me, although it's not strictly necessary since in Emacs itself, --apply func -- is basically equivalent to --eval (apply func command-line-args-left) (This doesn't work in emacsclient, and would be very difficult to support, so --apply is necessary for emacsclient) > However, if people can't agree, then we could probably drop that > part. To me, it sounds like people *do* agree that this would be good > to have though. I'm fine either way. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 20:00 ` Spencer Baugh @ 2023-09-13 20:16 ` Jim Porter 0 siblings, 0 replies; 59+ messages in thread From: Jim Porter @ 2023-09-13 20:16 UTC (permalink / raw) To: Spencer Baugh; +Cc: sbaugh, Eli Zaretskii, 65902 On 9/13/2023 1:00 PM, Spencer Baugh wrote: > It doesn't work with --alternate-editor=emacs, but I think that is OK. > I think the alternate-editor='' configuration is far more common, and > that works just fine. In that case, I think the benefits of "emacs --apply" are somewhat reduced, but it does still make some cases a bit easier. ("emacs-mail.desktop" could use it, which would let us simplify the implementation of 'message-mailto' a little.) Still, if we dropped "emacs --apply" I wouldn't stamp my feet about it. The main benefit is for emacsclient anyway, where it seems we have consensus. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-13 19:33 ` Jim Porter 2023-09-13 20:00 ` Spencer Baugh @ 2023-09-14 5:10 ` Eli Zaretskii 2023-09-14 11:03 ` sbaugh 2023-09-16 13:43 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 2 replies; 59+ messages in thread From: Eli Zaretskii @ 2023-09-14 5:10 UTC (permalink / raw) To: Jim Porter; +Cc: 65902, sbaugh, sbaugh > Date: Wed, 13 Sep 2023 12:33:01 -0700 > Cc: sbaugh@catern.com, 65902@debbugs.gnu.org > From: Jim Porter <jporterbugs@gmail.com> > > On 9/13/2023 12:13 PM, Eli Zaretskii wrote: > > And I don't understand why we need to add any options to Emacs itself, > > btw. The suggestion to have some "symmetry" here was one of the > > reasons that discussion got nowhere. So let's learn from that > > mistake, at least. > > There's a practical benefit to this. If you have $EDITOR set in your > environment to something like "emacsclient --alternate-editor=emacs", > then it would be nice if you could say this: > > $EDITOR --apply some-func arg1 arg2 > > and have it do the same thing whether or not there was already an Emacs > server running. The symmetry between the two commands (plus proper > argument forwarding) would make that work. > > However, if people can't agree, then we could probably drop that part. > To me, it sounds like people *do* agree that this would be good to have > though. People might agree, but I don't. Please consider the perspective. This started as an obscure and rare problem in a desktop file (which we provided solely out of good will, since it really isn't our job to do so, it's the job of downstream distros). The proposed solution was to add a completely new option to emacsclient, with its own special syntax and rules about what can and cannot be done with it. This is already something that should raise brows: how can such an unimportant reason cause us to make such significant changes? We didn't yet finish discussing that nor even had time to understand all the implications (remember: Lars suggested to support several such options, which required another special option), and we already are told that "for symmetry" we should add the same to Emacs. All that where just yesterday there was no need for any new options in either, and if we decided to drop those desktop files from our sources (which I personally am tempted to do every few weeks, due to issues they cause us all the time since their introduction), then even the original need will miraculously disappear into thin air. So this is a classic case of the tail wagging the dog. What about alternative solutions: use a shell script in the desktop files, and delegate to that script to solve the problem with quoting? Had anyone considered this strategy? If not, why not? I would in general prefer not to add any new options to our programs due to this weak reason. Once again: it is not our job to get these desktop files right in every single downstream environment, so let's not make it our problem, certainly not a problem we should solve using such non-trivial solutions. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 5:10 ` Eli Zaretskii @ 2023-09-14 11:03 ` sbaugh 2023-09-14 11:18 ` sbaugh ` (2 more replies) 2023-09-16 13:43 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 3 replies; 59+ messages in thread From: sbaugh @ 2023-09-14 11:03 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Jim Porter, sbaugh, 65902 Eli Zaretskii <eliz@gnu.org> writes: >> Date: Wed, 13 Sep 2023 12:33:01 -0700 >> Cc: sbaugh@catern.com, 65902@debbugs.gnu.org >> From: Jim Porter <jporterbugs@gmail.com> >> >> On 9/13/2023 12:13 PM, Eli Zaretskii wrote: >> > And I don't understand why we need to add any options to Emacs itself, >> > btw. The suggestion to have some "symmetry" here was one of the >> > reasons that discussion got nowhere. So let's learn from that >> > mistake, at least. >> >> There's a practical benefit to this. If you have $EDITOR set in your >> environment to something like "emacsclient --alternate-editor=emacs", >> then it would be nice if you could say this: >> >> $EDITOR --apply some-func arg1 arg2 >> >> and have it do the same thing whether or not there was already an Emacs >> server running. The symmetry between the two commands (plus proper >> argument forwarding) would make that work. >> >> However, if people can't agree, then we could probably drop that part. >> To me, it sounds like people *do* agree that this would be good to have >> though. > > People might agree, but I don't. Please consider the perspective. > This started as an obscure and rare problem in a desktop file (which > we provided solely out of good will, since it really isn't our job to > do so, it's the job of downstream distros). The proposed solution was > to add a completely new option to emacsclient, with its own special > syntax and rules about what can and cannot be done with it. This is > already something that should raise brows: how can such an unimportant > reason cause us to make such significant changes? We didn't yet > finish discussing that nor even had time to understand all the > implications (remember: Lars suggested to support several such > options, which required another special option), and we already are > told that "for symmetry" we should add the same to Emacs. All that > where just yesterday there was no need for any new options in either, > and if we decided to drop those desktop files from our sources (which > I personally am tempted to do every few weeks, due to issues they > cause us all the time since their introduction), then even the > original need will miraculously disappear into thin air. The issue is not really with the desktop file. It's a generic problem: Suppose I have some arbitrary data which I want to send to the Emacs server (in this case, a URI). Today, there's no easy way to do that. - One approach is to stick the data inside an --eval call, as emacsclient-mail.desktop is doing. Getting the quoting right is hard and complex, and even Emacs developers have failed at it over multiple iterations, and when they fail it either breaks or exposes a security vulnerability. It is currently broken for me. - Another approach is to do what org-protocol does (shipped with Emacs!), and advise server-visit-files (with org-protocol-check-filename-for-protocol), and pass the data as a string FILE argument to emacsclient which gets intercepted by advice. But this is of course a gross hack, and also it still requires escaping the data, since some characters will still be specially interpreted. It would be nice to get rid of this org-protocol hack which is shipped with Emacs. - A third approach is to put the data in a temporary file and pass the path of that file to emacsclient, then use an --eval to process the file. But this doesn't work when emacsclient and the Emacs server are on different hosts or in different environments. - Finally, a fourth approach is to teach emacsclient to be able to send this arbitrary data to Emacs on its own. Such as by adding --apply. There are a bunch of designs for that, but all of them require modifying emacsclient. I have many times before wanted to be able to pass data to Emacs without worrying about escaping. This would be a very useful feature to have. And it would be nice to get rid of the org-protocol hack, and get rid of the complicated and broken escaping needed for things like emacsclient-mail.desktop. > So this is a classic case of the tail wagging the dog. > > What about alternative solutions: use a shell script in the desktop > files, and delegate to that script to solve the problem with quoting? > Had anyone considered this strategy? If not, why not? Getting the quoting right is hard and complex, and even Emacs developers have failed at it over multiple iterations, and when they fail it either breaks or exposes a security vulnerability. This solution is far simpler, and is reusable for many other different purposes. > I would in general prefer not to add any new options to our programs > due to this weak reason. Once again: it is not our job to get these > desktop files right in every single downstream environment, so let's > not make it our problem, certainly not a problem we should solve using > such non-trivial solutions. We don't have to add it to Emacs itself. But it's not just for the desktop files. It's a relatively small feature to add to emacsclient, and it's something that I've heard years of user requests for. I have many times before wanted to be able to pass data to Emacs without worrying about escaping. This would be a very useful feature to have. It would be nice to get rid of the org-protocol hack, and get rid of the complicated and broken escaping needed for things like emacsclient-mail.desktop. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 11:03 ` sbaugh @ 2023-09-14 11:18 ` sbaugh 2023-09-14 11:35 ` sbaugh 2023-09-14 13:36 ` Eli Zaretskii 2 siblings, 0 replies; 59+ messages in thread From: sbaugh @ 2023-09-14 11:18 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, Jim Porter, sbaugh [-- Attachment #1: Type: text/plain, Size: 115 bytes --] Here's a revised patch which drops adding the argument to Emacs itself; it only adds an argument to emacsclient. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-apply-argument-to-emacsclient-to-avoid-escaping.patch --] [-- Type: text/x-patch, Size: 7960 bytes --] From d4558462598eaa33466b3b2fb1dbcc26ea877ede Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Thu, 14 Sep 2023 07:12:22 -0400 Subject: [PATCH] Add --apply argument to emacsclient to avoid escaping Passing arguments to functions through emacsclient --eval requires complicated escaping (as seen in emacsclient-mail.desktop before this change). The new --apply argument for emacsclient passes command line arguments as uninterpreted strings to the specified function. This simplifies use cases where arbitrary input needs to be passed to Emacs. org-protocol will be able to use this as well, which will allow it to eventually drop its current advice on server-visit-files. * etc/emacsclient-mail.desktop: Use --apply. (bug#65902) * lib-src/emacsclient.c (longopts, decode_options, main): Add support for --apply. * lisp/server.el (server-apply-and-print): Add. (server-process-filter): Add support for -apply and -applyargs --- etc/emacsclient-mail.desktop | 7 ++----- lib-src/emacsclient.c | 36 ++++++++++++++++++++++++++++++++++-- lisp/server.el | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..ea0690bacd9 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --apply message-mailto -- %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,7 +13,7 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --apply message-mailto -- %u [Desktop Action new-instance] Name=New Instance diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c index 698bf9b50ae..159c22d1ae9 100644 --- a/lib-src/emacsclient.c +++ b/lib-src/emacsclient.c @@ -116,6 +116,9 @@ #define DEFAULT_TIMEOUT (30) /* True means args are expressions to be evaluated. --eval. */ static bool eval; +/* The function to call. Other arguments are passed as strings. --apply. */ +static char *apply; + /* True means open a new frame. --create-frame etc. */ static bool create_frame; @@ -169,6 +172,7 @@ #define DEFAULT_TIMEOUT (30) { "quiet", no_argument, NULL, 'q' }, { "suppress-output", no_argument, NULL, 'u' }, { "eval", no_argument, NULL, 'e' }, + { "apply", required_argument, NULL, 'y' }, { "help", no_argument, NULL, 'H' }, { "version", no_argument, NULL, 'V' }, { "tty", no_argument, NULL, 't' }, @@ -552,6 +556,10 @@ decode_options (int argc, char **argv) eval = true; break; + case 'y': + apply = optarg; + break; + case 'q': quiet = true; break; @@ -690,6 +698,7 @@ print_help_and_exit (void) -F ALIST, --frame-parameters=ALIST\n\ Set the parameters of a new frame\n\ -e, --eval Evaluate the FILE arguments as ELisp expressions\n\ +-y, --apply FUNC Call ELisp FUNC, passing all FILE arguments as strings\n\ -n, --no-wait Don't wait for the server to return\n\ -w, --timeout=SECONDS Seconds to wait before timing out\n\ -q, --quiet Don't display messages on success\n\ @@ -1953,7 +1962,7 @@ main (int argc, char **argv) /* Process options. */ decode_options (argc, argv); - if (! (optind < argc || eval || create_frame)) + if (! (optind < argc || eval || apply || create_frame)) { message (true, ("%s: file name or argument required\n" "Try '%s --help' for more information\n"), @@ -1961,6 +1970,14 @@ main (int argc, char **argv) exit (EXIT_FAILURE); } + if (eval && apply) + { + message (true, ("%s: can't pass both --eval and --apply\n" + "Try '%s --help' for more information\n"), + progname, progname); + exit (EXIT_FAILURE); + } + #ifdef SOCKETS_IN_FILE_SYSTEM if (tty) { @@ -2080,6 +2097,13 @@ main (int argc, char **argv) send_to_emacs (emacs_socket, " "); continue; } + else if (apply) + { + send_to_emacs (emacs_socket, "-applyarg "); + quote_argument (emacs_socket, argv[i]); + send_to_emacs (emacs_socket, " "); + continue; + } char *p = argv[i]; if (*p == '+') @@ -2136,10 +2160,18 @@ main (int argc, char **argv) send_to_emacs (emacs_socket, " "); } + if (apply) + { + send_to_emacs (emacs_socket, "-apply "); + quote_argument (emacs_socket, apply); + send_to_emacs (emacs_socket, " "); + } + + send_to_emacs (emacs_socket, "\n"); /* Wait for an answer. */ - if (!eval && !tty && !nowait && !quiet && 0 <= process_grouping ()) + if (!eval && !apply && !tty && !nowait && !quiet && 0 <= process_grouping ()) { printf ("Waiting for Emacs..."); skiplf = false; diff --git a/lisp/server.el b/lisp/server.el index c3325e5a24c..5981e90625d 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -873,6 +873,17 @@ server-eval-and-print (point-min) (point-max)))) (server-reply-print (server-quote-arg text) proc))))))) +(defun server-apply-and-print (func args proc) + "Call FUNC on ARGS and send the result back to client PROC." + (let ((v (with-local-quit (eval (apply (intern func) args) t)))) + (when proc + (with-temp-buffer + (let ((standard-output (current-buffer))) + (pp v) + (let ((text (buffer-substring-no-properties + (point-min) (point-max)))) + (server-reply-print (server-quote-arg text) proc))))))) + (defconst server-msg-size 1024 "Maximum size of a message sent to a client.") @@ -1196,6 +1207,7 @@ server-process-filter tty-type ; string. files filepos + applyargs args-left) ;; Remove this line from STRING. (setq string (substring string (match-end 0))) @@ -1323,6 +1335,28 @@ server-process-filter commands) (setq filepos nil))) + ;; -apply FUNC: Call a function on arguments. + ("-apply" + (if use-current-frame + (setq use-current-frame 'always)) + (let ((func (pop args-left))) + (if coding-system + (setq func (decode-coding-string func coding-system))) + (push (lambda () (server-apply-and-print func applyargs proc)) + commands) + (setq applyargs nil) + (setq filepos nil))) + + ;; -applyarg ARG: Add an argument for later -apply. + ("-applyarg" + (if use-current-frame + (setq use-current-frame 'always)) + (let ((arg (pop args-left))) + (if coding-system + (setq arg (decode-coding-string arg coding-system))) + (push arg applyargs) + (setq filepos nil))) + ;; -env NAME=VALUE: An environment variable. ("-env" (let ((var (pop args-left))) -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 11:03 ` sbaugh 2023-09-14 11:18 ` sbaugh @ 2023-09-14 11:35 ` sbaugh 2023-09-14 13:36 ` Eli Zaretskii 2 siblings, 0 replies; 59+ messages in thread From: sbaugh @ 2023-09-14 11:35 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, Jim Porter, sbaugh sbaugh@catern.com writes: > - Finally, a fourth approach is to teach emacsclient to be able to send > this arbitrary data to Emacs on its own. Such as by adding --apply. > There are a bunch of designs for that, but all of them require > modifying emacsclient. BTW, if this specific approach of --apply seems inelegant to you, I could make a different design work. As long as Lisp code gets direct access to the arguments as strings, without needing escaping. One person in the previous thread proposed making a command-line-args-left equivalent for emacsclient. If we did this, it wouldn't require adding any new arguments to the emacsclient interface. This could be called server-eval-args-left, so that --apply could be done instead as: emacsclient --eval '(progn (apply func server-eval-args-left) (setq server-eval-args-left nil))' arg1 arg2 I can do that if you prefer that, but it will require some more significant changes to the internals (because currently server.el processes -eval commands one at a time and doesn't have access to the subsequent ones). ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 11:03 ` sbaugh 2023-09-14 11:18 ` sbaugh 2023-09-14 11:35 ` sbaugh @ 2023-09-14 13:36 ` Eli Zaretskii 2023-09-14 14:04 ` Spencer Baugh 2023-09-14 19:16 ` Jim Porter 2 siblings, 2 replies; 59+ messages in thread From: Eli Zaretskii @ 2023-09-14 13:36 UTC (permalink / raw) To: sbaugh; +Cc: jporterbugs, sbaugh, 65902 > From: sbaugh@catern.com > Date: Thu, 14 Sep 2023 11:03:44 +0000 (UTC) > Cc: Jim Porter <jporterbugs@gmail.com>, 65902@debbugs.gnu.org, > sbaugh@janestreet.com > > The issue is not really with the desktop file. It's a generic problem: No, it isn't. If it were, we'd have heard about it much sooner, and not because of the desktop files. What you are doing is representing a rare problem related to a niche feature is if it were a general one, by inventing use cases to justify that. But if those use cases were important, people would have asked for them long ago. They didn't. Why? because --eval already exists. > > What about alternative solutions: use a shell script in the desktop > > files, and delegate to that script to solve the problem with quoting? > > Had anyone considered this strategy? If not, why not? > > Getting the quoting right is hard and complex, and even Emacs developers > have failed at it over multiple iterations, and when they fail it either > breaks or exposes a security vulnerability. Emacs developers make mistakes even in the simple regexps we have in our code. That doesn't mean we should abandon regexps. The solution for sending Lisp forms to the server exists, and the quoting, although tricky in some cases, is not rocket science to get right. I don't see why we would need another mechanism to do something similar with radically different syntax, a separate set of rules and restrictions that need to be documented, etc. etc. > This solution is far simpler That's an illusion. There's nothing simple about it. You are inventing a new mechanism for passing Lisp forms as something other than Lisp. This has got to have issues into which we will bump sooner or later. E.g., assume that two or more of the arguments to the function begins with single quote, as in $ emacsclient --apply func arg1 'foo arg2 'bar Escape-quoting, here we come again! ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 13:36 ` Eli Zaretskii @ 2023-09-14 14:04 ` Spencer Baugh 2023-09-14 14:31 ` Eli Zaretskii 2023-09-14 19:16 ` Jim Porter 1 sibling, 1 reply; 59+ messages in thread From: Spencer Baugh @ 2023-09-14 14:04 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, sbaugh, jporterbugs Eli Zaretskii <eliz@gnu.org> writes: >> From: sbaugh@catern.com >> Date: Thu, 14 Sep 2023 11:03:44 +0000 (UTC) >> Cc: Jim Porter <jporterbugs@gmail.com>, 65902@debbugs.gnu.org, >> sbaugh@janestreet.com >> >> The issue is not really with the desktop file. It's a generic problem: > > No, it isn't. If it were, we'd have heard about it much sooner, and > not because of the desktop files. > > What you are doing is representing a rare problem related to a niche > feature is if it were a general one, by inventing use cases to justify > that. But if those use cases were important, people would have asked > for them long ago. They didn't. Why? because --eval already exists. No... these are real use cases that I personally have. I have really wanted this for a long time. As I said in my original email, "I expect this to also be useful in other places; the need to escape arbitrary inputs before passing them to emacsclient is frequently annoying." - I've wanted the ability to pass arbitrary data to Emacs through emacsclient since at least 2016, for other reasons: https://lists.gnu.org/archive/html/emacs-devel/2016-06/msg00051.html - The fact that there is currently advice in org-protocol to implement this behavior means there are people who want it. I'm sure the org-protocol authors really wanted to be able to avoid that advice, back when they developed it in 2009. They didn't bother contributing a solution upstream back then, but why stop it from happening now? - It would allow lazy loading of org-protocol as desired by the org devs https://list.orgmode.org/strc07$3o0$1@ciao.gmane.io - I'm working on a package which allows using Emacs to do completing-read over arbitrary strings passed in from the command line, as a replacement for the popular terminal software fzf. Since this is completion over arbitrary strings, I need the ability to get those arbitrary strings into Emacs. - There are numerous examples on the web of users trying and failing to get the quoting right to pass arguments to emacsclient; for example https://www.reddit.com/r/emacs/comments/hhbcg7/emacsclient_eval_with_command_line_arguments/ this would become emacsclient --apply switch-to-buffer https://stackoverflow.com/questions/8848819/emacs-eval-ediff-1-2-how-to-put-this-line-in-to-shell-script this would become emacsclient --apply ediff No shell complexities required in either case. >> > What about alternative solutions: use a shell script in the desktop >> > files, and delegate to that script to solve the problem with quoting? >> > Had anyone considered this strategy? If not, why not? >> >> Getting the quoting right is hard and complex, and even Emacs developers >> have failed at it over multiple iterations, and when they fail it either >> breaks or exposes a security vulnerability. > > Emacs developers make mistakes even in the simple regexps we have in > our code. That doesn't mean we should abandon regexps. The solution > for sending Lisp forms to the server exists, and the quoting, although > tricky in some cases, is not rocket science to get right. I think this (the current contents of emacsclient-mail.desktop): sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u is in fact rocket science, and rocket science that needs to be repeated by every user who wants to pass arbitrary strings to Emacs. And keep in mind this mass of escaping *is currently broken*. > I don't see > why we would need another mechanism to do something similar with > radically different syntax, a separate set of rules and restrictions > that need to be documented, etc. etc. > >> This solution is far simpler > > That's an illusion. There's nothing simple about it. You are > inventing a new mechanism for passing Lisp forms as something other > than Lisp. But I don't want to pass Lisp forms, that's the entire point. I have some arbitrary string which is *not* Lisp, and I want Emacs to *not* parse it as Lisp. > This has got to have issues into which we will bump sooner > or later. E.g., assume that two or more of the arguments to the > function begins with single quote, as in > > $ emacsclient --apply func arg1 'foo arg2 'bar > > Escape-quoting, here we come again! That example works fine with --apply. The call becomes: (func "arg1" "'foo" "arg2" "'bar") which is reliable and expected. Maybe you're referring to how, if you run that command through a shell, the shell interprets the single quotes as creating a string? But that's that's a separate issue, because: - I don't plan to run any of my commands using --apply through a shell (which means they will require zero escaping or quoting whatsoever) - Right now with --eval you have to do escaping for both the shell and Lisp. With --apply you only have to do escaping for the shell, if you do use a shell, and if you don't use a shell you don't have to do anything. I think it is simpler to reduce the amount of quoting and escaping from "both Lisp and shell" to "just shell, and not even that if you don't use a shell". ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 14:04 ` Spencer Baugh @ 2023-09-14 14:31 ` Eli Zaretskii 0 siblings, 0 replies; 59+ messages in thread From: Eli Zaretskii @ 2023-09-14 14:31 UTC (permalink / raw) To: Spencer Baugh; +Cc: 65902, sbaugh, jporterbugs > From: Spencer Baugh <sbaugh@janestreet.com> > Cc: sbaugh@catern.com, jporterbugs@gmail.com, 65902@debbugs.gnu.org > Date: Thu, 14 Sep 2023 10:04:48 -0400 > > Eli Zaretskii <eliz@gnu.org> writes: > > > What you are doing is representing a rare problem related to a niche > > feature is if it were a general one, by inventing use cases to justify > > that. But if those use cases were important, people would have asked > > for them long ago. They didn't. Why? because --eval already exists. > > No... these are real use cases that I personally have. I have really > wanted this for a long time. As I said in my original email, "I expect > this to also be useful in other places; the need to escape arbitrary > inputs before passing them to emacsclient is frequently annoying." Maybe it's annoying, but it can be done. And Emacs has the same feature, btw. > > Emacs developers make mistakes even in the simple regexps we have in > > our code. That doesn't mean we should abandon regexps. The solution > > for sending Lisp forms to the server exists, and the quoting, although > > tricky in some cases, is not rocket science to get right. > > I think this (the current contents of emacsclient-mail.desktop): > sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec > emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval > \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u > > is in fact rocket science, and rocket science that needs to be repeated > by every user who wants to pass arbitrary strings to Emacs. We disagree. > And keep in mind this mass of escaping *is currently broken*. Patches to fix it are welcome, although as I said I'd be quite glad to remove these desktop files from our repository. > > That's an illusion. There's nothing simple about it. You are > > inventing a new mechanism for passing Lisp forms as something other > > than Lisp. > > But I don't want to pass Lisp forms, that's the entire point. I have > some arbitrary string which is *not* Lisp, and I want Emacs to *not* > parse it as Lisp. It becomes Lisp when the server executes the request. > > $ emacsclient --apply func arg1 'foo arg2 'bar > > > > Escape-quoting, here we come again! > > That example works fine with --apply. The call becomes: > (func "arg1" "'foo" "arg2" "'bar") > which is reliable and expected. > > Maybe you're referring to how, if you run that command through a shell, > the shell interprets the single quotes as creating a string? Of course, I am! > But that's that's a separate issue, because: > > - I don't plan to run any of my commands using --apply through a shell > (which means they will require zero escaping or quoting whatsoever) This feature, if it will be added, is not just for you, it's for everyone. And emacsclient is a shell command, so invoking it from the shell is both natural and frequently used. > - Right now with --eval you have to do escaping for both the shell and > Lisp. With --apply you only have to do escaping for the shell, if you > do use a shell, and if you don't use a shell you don't have to do > anything. But we do that for Emacs, and do it quite a lot. > I think it is simpler to reduce the amount of quoting and escaping from > "both Lisp and shell" to "just shell, and not even that if you don't use > a shell". At what cost? The cost of adding yet another protocol for passing Lisp forms to the server is just too high for my palate. Bottom line: the escaping issue doesn't seem to me a reason strong enough to justify adding such a new feature. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 13:36 ` Eli Zaretskii 2023-09-14 14:04 ` Spencer Baugh @ 2023-09-14 19:16 ` Jim Porter 2023-09-15 5:33 ` Eli Zaretskii 1 sibling, 1 reply; 59+ messages in thread From: Jim Porter @ 2023-09-14 19:16 UTC (permalink / raw) To: Eli Zaretskii, sbaugh; +Cc: sbaugh, 65902 On 9/14/2023 6:36 AM, Eli Zaretskii wrote: >> From: sbaugh@catern.com >> Date: Thu, 14 Sep 2023 11:03:44 +0000 (UTC) >> Cc: Jim Porter <jporterbugs@gmail.com>, 65902@debbugs.gnu.org, >> sbaugh@janestreet.com >> >> The issue is not really with the desktop file. It's a generic problem: > > No, it isn't. If it were, we'd have heard about it much sooner, and > not because of the desktop files. This actually *has* come up before, but in Org. "org-protocol.el" tries to solves a similar use case (use emacsclient to open an "org-protocol:" URL, which calls some Org-mode function). The code for this integration is quite complex though, and relies on advising 'server-visit-files' (see the code around 'org--protocol-detect-protocol-server' in lisp/org/org-protocol.el). Overall, the Org solution works, though it's hacky, and hooks into things a bit too late, so Org has to do extra work to clean up the arguments it receives. For example, on MS-Windows[1], instead of seeing the original "org-protocol:/..." string that gets passed to emacsclient, Org sees something like "c:/WINDOWS/system32/org-protocol:/...". Org works around this problem, but it would be nice if there were a way to hook into things earlier before the input arguments had been munged. There have been a few reports/complaints about how this works on the Org list in the past, too. I'm not married to any particular implementation for this problem, but given how this has come up for multiple cases (admittedly both for handling URLs of some form), I think there's value in considering a common way to handle this that's more straightforward than shell scripting or org-protocol's advice hacks. [1] This is also an example of how the problem isn't *just* with desktop files, since MS-Windows doesn't use those; instead, you have to set some registry keys. Otherwise, the problems are pretty similar to desktop files though. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 19:16 ` Jim Porter @ 2023-09-15 5:33 ` Eli Zaretskii 0 siblings, 0 replies; 59+ messages in thread From: Eli Zaretskii @ 2023-09-15 5:33 UTC (permalink / raw) To: Jim Porter; +Cc: 65902, sbaugh, sbaugh > Date: Thu, 14 Sep 2023 12:16:17 -0700 > Cc: sbaugh@janestreet.com, 65902@debbugs.gnu.org > From: Jim Porter <jporterbugs@gmail.com> > > [1] This is also an example of how the problem isn't *just* with desktop > files, since MS-Windows doesn't use those; instead, you have to set some > registry keys. Otherwise, the problems are pretty similar to desktop > files though. No, it isn't: on Windows you configure all kinds of desktop shortcuts to invoke Emacs or emacsclient, and those shortcuts allow you to include %x style parameters inside quotes. In addition, there's no need to use the "sh -c COMMAND" style of invoking commands. So most of the quoting issues mentioned here as the main reason for the change don't arise at all. But my problem is not with solving the Windows case, so let's drop this tangent. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 5:10 ` Eli Zaretskii 2023-09-14 11:03 ` sbaugh @ 2023-09-16 13:43 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-09-16 14:02 ` Eli Zaretskii 1 sibling, 1 reply; 59+ messages in thread From: Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-09-16 13:43 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Jim Porter, sbaugh, sbaugh, 65902 Eli Zaretskii <eliz@gnu.org> writes: >> Date: Wed, 13 Sep 2023 12:33:01 -0700 >> Cc: sbaugh@catern.com, 65902@debbugs.gnu.org >> From: Jim Porter <jporterbugs@gmail.com> >> >> On 9/13/2023 12:13 PM, Eli Zaretskii wrote: >> > And I don't understand why we need to add any options to Emacs itself, >> > btw. The suggestion to have some "symmetry" here was one of the >> > reasons that discussion got nowhere. So let's learn from that >> > mistake, at least. >> >> There's a practical benefit to this. If you have $EDITOR set in your >> environment to something like "emacsclient --alternate-editor=emacs", >> then it would be nice if you could say this: >> >> $EDITOR --apply some-func arg1 arg2 >> >> and have it do the same thing whether or not there was already an Emacs >> server running. The symmetry between the two commands (plus proper >> argument forwarding) would make that work. >> >> However, if people can't agree, then we could probably drop that part. >> To me, it sounds like people *do* agree that this would be good to have >> though. > > People might agree, but I don't. Please consider the perspective. > This started as an obscure and rare problem in a desktop file (which > we provided solely out of good will, since it really isn't our job to > do so, it's the job of downstream distros). Programs ship their own desktop files. It is not the job of the downstream to provide desktop files unless it is because of a patch from said downstream. In any case it doesn't make sense for each downstream to ship their own desktop file and fix their own. From my pov letting Emacs do all work in passing arguments is the best solution as escaping is error prone. I haven't seen any program besides Emacs using escaped shell syntax to pass the escaped syntax of their target language. Most just have options to pass the target file or deal with the dbus activation/interface. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-16 13:43 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-09-16 14:02 ` Eli Zaretskii 2023-09-16 15:54 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-16 14:02 UTC (permalink / raw) To: Björn Bidar; +Cc: jporterbugs, sbaugh, sbaugh, 65902 > From: Björn Bidar <bjorn.bidar@thaodan.de> > Cc: Jim Porter <jporterbugs@gmail.com>, 65902@debbugs.gnu.org, > sbaugh@janestreet.com, sbaugh@catern.com > Date: Sat, 16 Sep 2023 16:43:20 +0300 > > Eli Zaretskii <eliz@gnu.org> writes: > > > People might agree, but I don't. Please consider the perspective. > > This started as an obscure and rare problem in a desktop file (which > > we provided solely out of good will, since it really isn't our job to > > do so, it's the job of downstream distros). > > Programs ship their own desktop files. Programs are shipped by distros. Emacs as a project does not ship anything, we just release the sources in a form that makes building Emacs easy. Our source tarballs cannot be installed, so they are not a finished, ready-to-use product. Distros, by contrast, do ship programs ready to be used. > It is not the job of the downstream to provide desktop files unless > it is because of a patch from said downstream. > > In any case it doesn't make sense for each downstream to ship their own > desktop file and fix their own. I disagree. Desktop files are specific to the target OS, but Emacs as the project does not target any specific OS (although there are OSes that we treat more favorably when considering features). The know-how about what exactly is needed for the desktop integration is also something that the distros have and we don't, except by chance. Each distro targets a single OS, and so it is reasonable to expect them to arrange for the necessary desktop integration. For example, no one would expect us to provide desktop shortcuts for MS-Windows, except as a sign of good will and when we have the necessary expertise on board. > >From my pov letting Emacs do all work in passing arguments is the best > solution as escaping is error prone. I haven't seen any program besides > Emacs using escaped shell syntax to pass the escaped syntax of their > target language. Most just have options to pass the target file or deal > with the dbus activation/interface. We don't require any escapes except those needed by Lisp. Note that in this case, at least some of the escapes are because the desktop shortcut invokes emacsclient via "sh -c "COMMAND STRING", something that should be considered an Emacs problem or a problem we must solve. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-16 14:02 ` Eli Zaretskii @ 2023-09-16 15:54 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 59+ messages in thread From: Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-09-16 15:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, jporterbugs, sbaugh, sbaugh Eli Zaretskii <eliz@gnu.org> writes: >> From: Björn Bidar <bjorn.bidar@thaodan.de> >> Cc: Jim Porter <jporterbugs@gmail.com>, 65902@debbugs.gnu.org, >> sbaugh@janestreet.com, sbaugh@catern.com >> Date: Sat, 16 Sep 2023 16:43:20 +0300 >> >> Eli Zaretskii <eliz@gnu.org> writes: >> >> > People might agree, but I don't. Please consider the perspective. >> > This started as an obscure and rare problem in a desktop file (which >> > we provided solely out of good will, since it really isn't our job to >> > do so, it's the job of downstream distros). >> >> Programs ship their own desktop files. > > Programs are shipped by distros. Emacs as a project does not ship > anything, we just release the sources in a form that makes building > Emacs easy. Our source tarballs cannot be installed, so they are not > a finished, ready-to-use product. Distros, by contrast, do ship > programs ready to be used. > Ship source or programs the difference is in semantics. >> It is not the job of the downstream to provide desktop files unless >> it is because of a patch from said downstream. >> >> In any case it doesn't make sense for each downstream to ship their own >> desktop file and fix their own. > > I disagree. Desktop files are specific to the target OS, but Emacs as > the project does not target any specific OS (although there are OSes > that we treat more favorably when considering features). The know-how > about what exactly is needed for the desktop integration is also > something that the distros have and we don't, except by chance. Emacs contains many files specific to operating system or shared by operating systems (in the case of desktop files mostly about different Linux distributions that share 90% of their specific attributes). But again it's about semantics, if it really was about not having any operating specific parts in Emacs than why is there Windows or macOS specific functions in Emacs. > Each distro targets a single OS, and so it is reasonable to expect > them to arrange for the necessary desktop integration. For example, > no one would expect us to provide desktop shortcuts for MS-Windows, > except as a sign of good will and when we have the necessary expertise > on board. Emacs has operating specific code, for Windows or macOS even in low level interfaces and not just lisp code or accessories such as desktop shortcuts. Each distribution complies with XDG, all desktop file are XDG compliant. Emacs has desktop files that work with all distributions be it Linux or BSD. >> >From my pov letting Emacs do all work in passing arguments is the best >> solution as escaping is error prone. I haven't seen any program besides >> Emacs using escaped shell syntax to pass the escaped syntax of their >> target language. Most just have options to pass the target file or deal >> with the dbus activation/interface. > > We don't require any escapes except those needed by Lisp. > > Note that in this case, at least some of the escapes are because the > desktop shortcut invokes emacsclient via "sh -c "COMMAND STRING", > something that should be considered an Emacs problem or a problem we > must solve. I agree. In the desktop files it is assumed that X11 is used (check for DISPLAY variable to be set) which breaks (native) Wayland usage, but that is another issue. ^ permalink raw reply [flat|nested] 59+ messages in thread
[parent not found: <80d8aeb0-c9f1-410f-b83d-60f83ca5b3af@email.android.com>]
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping [not found] <80d8aeb0-c9f1-410f-b83d-60f83ca5b3af@email.android.com> @ 2023-09-14 14:57 ` Eli Zaretskii 2023-09-14 15:10 ` Spencer Baugh 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-14 14:57 UTC (permalink / raw) To: Spencer Baugh; +Cc: 65902, sbaugh, jporterbugs > Date: Thu, 14 Sep 2023 14:48:14 +0000 (UTC) > From: Spencer Baugh <sbaugh@catern.com> > Cc: Spencer Baugh <sbaugh@janestreet.com>, jporterbugs@gmail.com, > 65902@debbugs.gnu.org > > Okay, if I do this without making modifications to emacsclient.c or the server protocol, would that be > more acceptable? The approach I described in another email, with server-eval-args-left, all it adds is > a new variable. I cannot find the description of that approach. What did it say? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 14:57 ` Eli Zaretskii @ 2023-09-14 15:10 ` Spencer Baugh 2023-09-15 6:29 ` Eli Zaretskii 0 siblings, 1 reply; 59+ messages in thread From: Spencer Baugh @ 2023-09-14 15:10 UTC (permalink / raw) To: Eli Zaretskii; +Cc: jporterbugs, Spencer Baugh, 65902 Eli Zaretskii <eliz@gnu.org> writes: >> Date: Thu, 14 Sep 2023 14:48:14 +0000 (UTC) >> From: Spencer Baugh <sbaugh@catern.com> >> Cc: Spencer Baugh <sbaugh@janestreet.com>, jporterbugs@gmail.com, >> 65902@debbugs.gnu.org >> >> Okay, if I do this without making modifications to emacsclient.c or the server protocol, would that be >> more acceptable? The approach I described in another email, with server-eval-args-left, all it adds is >> a new variable. > > I cannot find the description of that approach. What did it say? We could make a command-line-args-left equivalent for emacsclient, called server-eval-args-left, which contains the FILE arguments passed to emacsclient as strings. This can be done without making any changes to emacsclient.c or the server protocol. Then the message-mailto use case would look like this: emacsclient --eval '(message-mailto (pop server-eval-args-left))' %u This would match how message-mailto uses (pop command-line-args-left) internally. This would work for all the use-cases I described before; I'd be very happy with this solution (actually, I'm starting to prefer it to --apply). And again, it doesn't change emacsclient.c or the server protocol. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-14 15:10 ` Spencer Baugh @ 2023-09-15 6:29 ` Eli Zaretskii 2023-09-22 1:36 ` sbaugh 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-15 6:29 UTC (permalink / raw) To: Spencer Baugh; +Cc: jporterbugs, sbaugh, 65902 > From: Spencer Baugh <sbaugh@janestreet.com> > Cc: Spencer Baugh <sbaugh@catern.com>, 65902@debbugs.gnu.org, > jporterbugs@gmail.com > Date: Thu, 14 Sep 2023 11:10:44 -0400 > > We could make a command-line-args-left equivalent for emacsclient, > called server-eval-args-left, which contains the FILE arguments passed > to emacsclient as strings. This can be done without making any changes > to emacsclient.c or the server protocol. Then the message-mailto > use case would look like this: > > emacsclient --eval '(message-mailto (pop server-eval-args-left))' %u > > This would match how message-mailto uses (pop command-line-args-left) > internally. > > This would work for all the use-cases I described before; I'd be very > happy with this solution (actually, I'm starting to prefer it to > --apply). And again, it doesn't change emacsclient.c or the server > protocol. This could perhaps be acceptable (although it's still rather kludgey, IMO), but are you sure you understand all the consequences? Currently, when emacsclient is invoked like this: $ emacsclient --eval '(func args)' foo bar we send to the server the following commands: -eval (func args) -eval foo -eval bar IOW, every command-line argument after --eval is treated as being implicitly preceded with --eval. With your proposal, how will the server know that some of "-eval foo" commands should cause foo to be added to server-eval-args-left instead of being evaluated as it does now? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-15 6:29 ` Eli Zaretskii @ 2023-09-22 1:36 ` sbaugh 2023-09-22 6:36 ` Eli Zaretskii 2023-09-22 7:05 ` Stefan Kangas 0 siblings, 2 replies; 59+ messages in thread From: sbaugh @ 2023-09-22 1:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, Spencer Baugh, jporterbugs [-- Attachment #1: Type: text/plain, Size: 1598 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: Spencer Baugh <sbaugh@janestreet.com> >> Cc: Spencer Baugh <sbaugh@catern.com>, 65902@debbugs.gnu.org, >> jporterbugs@gmail.com >> Date: Thu, 14 Sep 2023 11:10:44 -0400 >> >> We could make a command-line-args-left equivalent for emacsclient, >> called server-eval-args-left, which contains the FILE arguments passed >> to emacsclient as strings. This can be done without making any changes >> to emacsclient.c or the server protocol. Then the message-mailto >> use case would look like this: >> >> emacsclient --eval '(message-mailto (pop server-eval-args-left))' %u >> >> This would match how message-mailto uses (pop command-line-args-left) >> internally. >> >> This would work for all the use-cases I described before; I'd be very >> happy with this solution (actually, I'm starting to prefer it to >> --apply). And again, it doesn't change emacsclient.c or the server >> protocol. > > This could perhaps be acceptable (although it's still rather kludgey, > IMO), but are you sure you understand all the consequences? > Currently, when emacsclient is invoked like this: > > $ emacsclient --eval '(func args)' foo bar > > we send to the server the following commands: > > -eval (func args) > -eval foo > -eval bar > > IOW, every command-line argument after --eval is treated as being > implicitly preceded with --eval. > > With your proposal, how will the server know that some of "-eval foo" > commands should cause foo to be added to server-eval-args-left instead > of being evaluated as it does now? As in the attached patch. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-server-eval-args-left-to-server.el.patch --] [-- Type: text/x-patch, Size: 5866 bytes --] From 1f0ff370a23eee1fdf740527efa75c0ddbe625bb Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Thu, 21 Sep 2023 21:35:50 -0400 Subject: [PATCH] Add server-eval-args-left to server.el Passing arbitrary arguments to functions through emacsclient --eval requires complicated escaping to avoid them being parsed as Lisp (as seen in emacsclient-mail.desktop before this change). This new variable server-eval-args-left allows access to the arguments before they are parsed as Lisp. By removing arguments from the variable before they're parsed, a snippet of Lisp can consume arguments, as in emacsclient-mail.desktop. org-protocol might be able to use this as well, which might allow it to drop its current advice on server-visit-files. * etc/emacsclient-mail.desktop: Use server-eval-args-left. (bug#65902) * lisp/server.el (server-eval-args-left): Add. (server-process-filter, server-execute): Make -eval arguments available through server-eval-args-left. --- etc/emacsclient-mail.desktop | 7 ++----- lisp/server.el | 21 +++++++++++++-------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..5962fa1764c 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,7 +13,7 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --eval '(message-mailto (pop server-eval-args-left))' %u [Desktop Action new-instance] Name=New Instance diff --git a/lisp/server.el b/lisp/server.el index c3325e5a24c..6eff6ebe140 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -1165,7 +1165,7 @@ server-process-filter (when prev (setq string (concat prev string)) (process-put proc 'previous-string nil))) - (condition-case err + (condition-case-unless-debug err (progn (server-add-client proc) ;; Send our pid @@ -1189,6 +1189,7 @@ server-process-filter parent-id ; Window ID for XEmbed dontkill ; t if client should not be killed. commands + evalexprs dir use-current-frame frame-parameters ;parameters for newly created frame @@ -1319,8 +1320,7 @@ server-process-filter (let ((expr (pop args-left))) (if coding-system (setq expr (decode-coding-string expr coding-system))) - (push (lambda () (server-eval-and-print expr proc)) - commands) + (push expr evalexprs) (setq filepos nil))) ;; -env NAME=VALUE: An environment variable. @@ -1345,7 +1345,7 @@ server-process-filter ;; arguments, use an existing frame. (and nowait (not (eq tty-name 'window-system)) - (or files commands) + (or files commands evalexprs) (setq use-current-frame t)) (setq frame @@ -1394,7 +1394,7 @@ server-process-filter (let ((default-directory (if (and dir (file-directory-p dir)) dir default-directory))) - (server-execute proc files nowait commands + (server-execute proc files nowait commands evalexprs dontkill frame tty-name))))) (when (or frame files) @@ -1404,22 +1404,27 @@ server-process-filter ;; condition-case (t (server-return-error proc err)))) -(defun server-execute (proc files nowait commands dontkill frame tty-name) +(defvar server-eval-args-left) + +(defun server-execute (proc files nowait commands evalexprs dontkill frame tty-name) ;; This is run from timers and process-filters, i.e. "asynchronously". ;; But w.r.t the user, this is not really asynchronous since the timer ;; is run after 0s and the process-filter is run in response to the ;; user running `emacsclient'. So it is OK to override the - ;; inhibit-quit flag, which is good since `commands' (as well as + ;; inhibit-quit flag, which is good since `evalexprs' (as well as ;; find-file-noselect via the major-mode) can run arbitrary code, ;; including code that needs to wait. (with-local-quit (condition-case err (let ((buffers (server-visit-files files proc nowait))) (mapc 'funcall (nreverse commands)) + (let ((server-eval-args-left (nreverse evalexprs))) + (while server-eval-args-left + (server-eval-and-print (pop server-eval-args-left) proc))) ;; If we were told only to open a new client, obey ;; `initial-buffer-choice' if it specifies a file ;; or a function. - (unless (or files commands) + (unless (or files commands evalexprs) (let ((buf (cond ((stringp initial-buffer-choice) (find-file-noselect initial-buffer-choice)) -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 1:36 ` sbaugh @ 2023-09-22 6:36 ` Eli Zaretskii 2023-09-23 20:24 ` sbaugh 2023-09-22 7:05 ` Stefan Kangas 1 sibling, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-22 6:36 UTC (permalink / raw) To: sbaugh; +Cc: 65902, sbaugh, jporterbugs > From: sbaugh@catern.com > Date: Fri, 22 Sep 2023 01:36:47 +0000 (UTC) > Cc: Spencer Baugh <sbaugh@janestreet.com>, jporterbugs@gmail.com, > 65902@debbugs.gnu.org > > > Currently, when emacsclient is invoked like this: > > > > $ emacsclient --eval '(func args)' foo bar > > > > we send to the server the following commands: > > > > -eval (func args) > > -eval foo > > -eval bar > > > > IOW, every command-line argument after --eval is treated as being > > implicitly preceded with --eval. > > > > With your proposal, how will the server know that some of "-eval foo" > > commands should cause foo to be added to server-eval-args-left instead > > of being evaluated as it does now? > > As in the attached patch. I wish you'd accompanied the patch with some plain-text description of the idea, to make it easier to understand and to avoid unnecessary misunderstandings. IIUC, this kind of solution is fine by me, but the protocol of accessing and using server-eval-args-left in the Lisp expressions specified on the emacsclient command line should be well-documented to avoid any confusion and UB. Also, the patch includes an unrelated change: > --- a/lisp/server.el > +++ b/lisp/server.el > @@ -1165,7 +1165,7 @@ server-process-filter > (when prev > (setq string (concat prev string)) > (process-put proc 'previous-string nil))) > - (condition-case err > + (condition-case-unless-debug err ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 6:36 ` Eli Zaretskii @ 2023-09-23 20:24 ` sbaugh 2023-09-24 5:18 ` Eli Zaretskii 0 siblings, 1 reply; 59+ messages in thread From: sbaugh @ 2023-09-23 20:24 UTC (permalink / raw) To: Eli Zaretskii; +Cc: jporterbugs, 65902, sbaugh [-- Attachment #1: Type: text/plain, Size: 1769 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: sbaugh@catern.com >> Date: Fri, 22 Sep 2023 01:36:47 +0000 (UTC) >> Cc: Spencer Baugh <sbaugh@janestreet.com>, jporterbugs@gmail.com, >> 65902@debbugs.gnu.org >> >> > Currently, when emacsclient is invoked like this: >> > >> > $ emacsclient --eval '(func args)' foo bar >> > >> > we send to the server the following commands: >> > >> > -eval (func args) >> > -eval foo >> > -eval bar >> > >> > IOW, every command-line argument after --eval is treated as being >> > implicitly preceded with --eval. >> > >> > With your proposal, how will the server know that some of "-eval foo" >> > commands should cause foo to be added to server-eval-args-left instead >> > of being evaluated as it does now? >> >> As in the attached patch. > > I wish you'd accompanied the patch with some plain-text description of > the idea, to make it easier to understand and to avoid unnecessary > misunderstandings. Ah, well, the basic idea is that for each -eval request we receive in the protocol, we accumulate the argument as a string into a single list (instead of immediately turning it into a closure which calls server-eval-and-print). Then we let-bind server-eval-args-left to that list around the calls to to server-eval-and-print, and consume each argument from the list one by one. > IIUC, this kind of solution is fine by me, but the protocol of > accessing and using server-eval-args-left in the Lisp expressions > specified on the emacsclient command line should be well-documented to > avoid any confusion and UB. Added a docstring and included it in NEWS. I would have put it in the manual, but it seems rather niche to put in the Emacs manual and I didn't see a natural place to put it in the Emacs Lisp manual. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-server-eval-args-left-to-server.el.patch --] [-- Type: text/x-patch, Size: 7851 bytes --] From cf7fcda1374491cc73a751d5636f36a95b2e8669 Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Thu, 21 Sep 2023 21:35:50 -0400 Subject: [PATCH] Add server-eval-args-left to server.el Passing arbitrary arguments to functions through emacsclient --eval requires complicated escaping to avoid them being parsed as Lisp (as seen in emacsclient-mail.desktop before this change). This new variable server-eval-args-left allows access to the arguments before they are parsed as Lisp. By removing arguments from the variable before they're parsed, a snippet of Lisp can consume arguments, as in emacsclient-mail.desktop. org-protocol might be able to use this as well, which might allow it to drop its current advice on server-visit-files. * etc/emacsclient-mail.desktop: Use server-eval-args-left. (bug#65902) * lisp/server.el (server-eval-args-left): Add. (server-process-filter, server-execute): Make -eval arguments available through server-eval-args-left. * lisp/startup.el (argv): Mention server-eval-args-left in docstring. * etc/NEWS: Announce server-eval-args-left. --- etc/NEWS | 10 ++++++++++ etc/emacsclient-mail.desktop | 7 ++----- lisp/server.el | 35 ++++++++++++++++++++++++++++------- lisp/startup.el | 5 ++++- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index cd4c414a64c..de1d79fcb98 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -121,6 +121,16 @@ Anything following the symbol 'mode-line-format-right-align' in right-aligned to is controlled by the new user option 'mode-line-right-align-edge'. +** Emacs Server and Client + +--- +*** 'server-eval-args-left' can be used to pop subsequent eval args +When '--eval' is passed to emacsclient and Emacs is evaluating each +argument, this variable is set to those which have not yet been +evaluated. It can be used to 'pop' arguments to prevent them from +being evaluated, which is useful when those arguments contain +arbitrary data. + \f * Editing Changes in Emacs 30.1 diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..5962fa1764c 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,7 +13,7 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --eval '(message-mailto (pop server-eval-args-left))' %u [Desktop Action new-instance] Name=New Instance diff --git a/lisp/server.el b/lisp/server.el index c3325e5a24c..65f28cdea23 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -1189,6 +1189,7 @@ server-process-filter parent-id ; Window ID for XEmbed dontkill ; t if client should not be killed. commands + evalexprs dir use-current-frame frame-parameters ;parameters for newly created frame @@ -1319,8 +1320,7 @@ server-process-filter (let ((expr (pop args-left))) (if coding-system (setq expr (decode-coding-string expr coding-system))) - (push (lambda () (server-eval-and-print expr proc)) - commands) + (push expr evalexprs) (setq filepos nil))) ;; -env NAME=VALUE: An environment variable. @@ -1345,7 +1345,7 @@ server-process-filter ;; arguments, use an existing frame. (and nowait (not (eq tty-name 'window-system)) - (or files commands) + (or files commands evalexprs) (setq use-current-frame t)) (setq frame @@ -1394,7 +1394,7 @@ server-process-filter (let ((default-directory (if (and dir (file-directory-p dir)) dir default-directory))) - (server-execute proc files nowait commands + (server-execute proc files nowait commands evalexprs dontkill frame tty-name))))) (when (or frame files) @@ -1404,22 +1404,43 @@ server-process-filter ;; condition-case (t (server-return-error proc err)))) -(defun server-execute (proc files nowait commands dontkill frame tty-name) +(defvar server-eval-args-left nil + "List of eval args not yet processed. + +When the server receives a request to eval one or more strings, +it stores them in this variable. Then, until this variable is +nil, it `pop's a string from this variable and evaluates it with +`server-eval-and-print'. Adding or removing strings from this +variable during this process will affect what is evaluated. + +This allows an expression passed to \"emacsclient --eval\" to +consume one or more subsequent arguments before they're parsed or +evaluated, with (pop server-eval-args-left). This is useful if +those arguments are arbitrary strings which should not be +evaluated. + +See also `command-line-args-left' for a similar variable which +works for command line invocations of \"emacs\".") + +(defun server-execute (proc files nowait commands evalexprs dontkill frame tty-name) ;; This is run from timers and process-filters, i.e. "asynchronously". ;; But w.r.t the user, this is not really asynchronous since the timer ;; is run after 0s and the process-filter is run in response to the ;; user running `emacsclient'. So it is OK to override the - ;; inhibit-quit flag, which is good since `commands' (as well as + ;; inhibit-quit flag, which is good since `evalexprs' (as well as ;; find-file-noselect via the major-mode) can run arbitrary code, ;; including code that needs to wait. (with-local-quit (condition-case err (let ((buffers (server-visit-files files proc nowait))) (mapc 'funcall (nreverse commands)) + (let ((server-eval-args-left (nreverse evalexprs))) + (while server-eval-args-left + (server-eval-and-print (pop server-eval-args-left) proc))) ;; If we were told only to open a new client, obey ;; `initial-buffer-choice' if it specifies a file ;; or a function. - (unless (or files commands) + (unless (or files commands evalexprs) (let ((buf (cond ((stringp initial-buffer-choice) (find-file-noselect initial-buffer-choice)) diff --git a/lisp/startup.el b/lisp/startup.el index 7f601668369..f2c84751211 100644 --- a/lisp/startup.el +++ b/lisp/startup.el @@ -120,7 +120,10 @@ command-switch-alist "List of command-line args not yet processed. This is a convenience alias, so that one can write (pop argv) inside of --eval command line arguments in order to access -following arguments.")) +following arguments. + +See also `server-eval-args-left' for a similar variable which +works for invocations of \"emacsclient --eval\".")) (internal-make-var-non-special 'argv) (defvar command-line-args-left nil -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-23 20:24 ` sbaugh @ 2023-09-24 5:18 ` Eli Zaretskii 2023-09-24 14:20 ` sbaugh 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-24 5:18 UTC (permalink / raw) To: sbaugh; +Cc: jporterbugs, 65902, sbaugh > From: sbaugh@catern.com > Date: Sat, 23 Sep 2023 20:24:07 +0000 (UTC) > Cc: 65902@debbugs.gnu.org, sbaugh@janestreet.com, jporterbugs@gmail.com > > > IIUC, this kind of solution is fine by me, but the protocol of > > accessing and using server-eval-args-left in the Lisp expressions > > specified on the emacsclient command line should be well-documented to > > avoid any confusion and UB. > > Added a docstring and included it in NEWS. I would have put it in the > manual, but it seems rather niche to put in the Emacs manual and I > didn't see a natural place to put it in the Emacs Lisp manual. The natural place is in the Emacs user manual, in "emacsclient Options", where --eval is described. Where else? > Passing arbitrary arguments to functions through emacsclient --eval > requires complicated escaping to avoid them being parsed as Lisp (as > seen in emacsclient-mail.desktop before this change). > > This new variable server-eval-args-left allows access to the arguments > before they are parsed as Lisp. By removing arguments from the > variable before they're parsed, a snippet of Lisp can consume > arguments, as in emacsclient-mail.desktop. > > org-protocol might be able to use this as well, which might allow it > to drop its current advice on server-visit-files. The right place to keep this information is in the manual and the doc strings, not just in the Git commit log message. > +(defvar server-eval-args-left nil > + "List of eval args not yet processed. > + > +When the server receives a request to eval one or more strings, > +it stores them in this variable. Then, until this variable is > +nil, it `pop's a string from this variable and evaluates it with > +`server-eval-and-print'. Adding or removing strings from this > +variable during this process will affect what is evaluated. This describes the implementation rather than the intended usage. > +This allows an expression passed to \"emacsclient --eval\" to > +consume one or more subsequent arguments before they're parsed or > +evaluated, with (pop server-eval-args-left). This is useful if > +those arguments are arbitrary strings which should not be > +evaluated. This describes a way of using this, but without the important part: that any processed argument _must_ be popped, or it will be evaluated again. See the doc string of command-line-functions for what I have in mind. All in all, I think this should be rewritten in terms of how to use this, and the result moved to the Emacs manual, leaving just the minimum here. > +See also `command-line-args-left' for a similar variable which > +works for command line invocations of \"emacs\".") This "see also" is not useful, because the doc string of command-line-args-left is minimal and doesn't add any information (which is okay, since that variable is basically meant for internal Emacs handling of command-line arguments, unlike this one). > --- a/lisp/startup.el > +++ b/lisp/startup.el > @@ -120,7 +120,10 @@ command-switch-alist > "List of command-line args not yet processed. > This is a convenience alias, so that one can write (pop argv) > inside of --eval command line arguments in order to access > -following arguments.")) > +following arguments. > + > +See also `server-eval-args-left' for a similar variable which > +works for invocations of \"emacsclient --eval\".")) And neither is this, because the use cases of the two variables are almost completely unrelated. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-24 5:18 ` Eli Zaretskii @ 2023-09-24 14:20 ` sbaugh 2023-10-21 15:20 ` sbaugh 0 siblings, 1 reply; 59+ messages in thread From: sbaugh @ 2023-09-24 14:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: jporterbugs, sbaugh, 65902 [-- Attachment #1: Type: text/plain, Size: 4293 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: sbaugh@catern.com >> Date: Sat, 23 Sep 2023 20:24:07 +0000 (UTC) >> Cc: 65902@debbugs.gnu.org, sbaugh@janestreet.com, jporterbugs@gmail.com >> >> > IIUC, this kind of solution is fine by me, but the protocol of >> > accessing and using server-eval-args-left in the Lisp expressions >> > specified on the emacsclient command line should be well-documented to >> > avoid any confusion and UB. >> >> Added a docstring and included it in NEWS. I would have put it in the >> manual, but it seems rather niche to put in the Emacs manual and I >> didn't see a natural place to put it in the Emacs Lisp manual. > > The natural place is in the Emacs user manual, in "emacsclient > Options", where --eval is described. Where else? Ah true, obvious, I added it there. >> Passing arbitrary arguments to functions through emacsclient --eval >> requires complicated escaping to avoid them being parsed as Lisp (as >> seen in emacsclient-mail.desktop before this change). >> >> This new variable server-eval-args-left allows access to the arguments >> before they are parsed as Lisp. By removing arguments from the >> variable before they're parsed, a snippet of Lisp can consume >> arguments, as in emacsclient-mail.desktop. >> >> org-protocol might be able to use this as well, which might allow it >> to drop its current advice on server-visit-files. > > The right place to keep this information is in the manual and the doc > strings, not just in the Git commit log message. Done. >> +(defvar server-eval-args-left nil >> + "List of eval args not yet processed. >> + >> +When the server receives a request to eval one or more strings, >> +it stores them in this variable. Then, until this variable is >> +nil, it `pop's a string from this variable and evaluates it with >> +`server-eval-and-print'. Adding or removing strings from this >> +variable during this process will affect what is evaluated. > > This describes the implementation rather than the intended usage. Fixed. >> +This allows an expression passed to \"emacsclient --eval\" to >> +consume one or more subsequent arguments before they're parsed or >> +evaluated, with (pop server-eval-args-left). This is useful if >> +those arguments are arbitrary strings which should not be >> +evaluated. > > This describes a way of using this, but without the important part: > that any processed argument _must_ be popped, or it will be evaluated > again. See the doc string of command-line-functions for what I have > in mind. > > All in all, I think this should be rewritten in terms of how to use > this, and the result moved to the Emacs manual, leaving just the > minimum here. Done. >> +See also `command-line-args-left' for a similar variable which >> +works for command line invocations of \"emacs\".") > > This "see also" is not useful, because the doc string of > command-line-args-left is minimal and doesn't add any information > (which is okay, since that variable is basically meant for internal > Emacs handling of command-line arguments, unlike this one). Okay, fair, probably that should point at `argv' instead. >> --- a/lisp/startup.el >> +++ b/lisp/startup.el >> @@ -120,7 +120,10 @@ command-switch-alist >> "List of command-line args not yet processed. >> This is a convenience alias, so that one can write (pop argv) >> inside of --eval command line arguments in order to access >> -following arguments.")) >> +following arguments. >> + >> +See also `server-eval-args-left' for a similar variable which >> +works for invocations of \"emacsclient --eval\".")) > > And neither is this, because the use cases of the two variables are > almost completely unrelated. The docstring of argv says: This is a convenience alias, so that one can write (pop argv) inside of --eval command line arguments in order to access following arguments. That is exactly the same way this new variable is used. And in fact it's used for the exact same purpose in message-mailto. The reason "emacs" doesn't require complicated escaping is because message-mailto does (pop command-line-args-left) internally. I agree that argv/command-line-args-left has other use cases besides this one. But one of argv's use cases is the exact same use case of server-eval-args-left. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-server-eval-args-left-to-server.el.patch --] [-- Type: text/x-patch, Size: 8438 bytes --] From bd49b918e57363593660f605c3e0efc3d0155c2b Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Thu, 21 Sep 2023 21:35:50 -0400 Subject: [PATCH] Add server-eval-args-left to server.el Passing arbitrary arguments to functions through emacsclient --eval requires complicated escaping to avoid them being parsed as Lisp (as seen in emacsclient-mail.desktop before this change). This new variable server-eval-args-left allows access to the arguments before they are parsed as Lisp. By removing arguments from the variable before they're parsed, a snippet of Lisp can consume arguments, as in emacsclient-mail.desktop. org-protocol might be able to use this as well, which might allow it to drop its current advice on server-visit-files. * etc/emacsclient-mail.desktop: Use server-eval-args-left. (bug#65902) * lisp/server.el (server-eval-args-left): Add. (server-process-filter, server-execute): Make -eval arguments available through server-eval-args-left. * lisp/startup.el (argv): Mention server-eval-args-left in docstring. * etc/NEWS: Announce server-eval-args-left. * doc/emacs/misc.texi (emacsclient Options): Document server-eval-args-left. --- doc/emacs/misc.texi | 9 +++++++++ etc/NEWS | 10 ++++++++++ etc/emacsclient-mail.desktop | 7 ++----- lisp/server.el | 27 ++++++++++++++++++++------- lisp/startup.el | 5 ++++- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi index 7a88b7ef5e0..1ce1713b01c 100644 --- a/doc/emacs/misc.texi +++ b/doc/emacs/misc.texi @@ -2078,6 +2078,15 @@ emacsclient Options @command{emacsclient} are interpreted as a list of expressions to evaluate, @emph{not} as a list of files to visit. +@vindex server-eval-args-left +If you have arbitrary data which you want to provide as input to one +of your expressions, you can pass the data as another argument to +@command{emacsclient} and use @var{server-eval-args-left} in the +expression to access the data. Be careful to have your expression +remove the data from @var{server-eval-args-left} regardless of whether +your code succeeds, such as by using @code{pop}, otherwise Emacs will +attempt to evaluate the data as a Lisp expression. + @item -f @var{server-file} @itemx --server-file=@var{server-file} Specify a server file (@pxref{TCP Emacs server}) for connecting to an diff --git a/etc/NEWS b/etc/NEWS index cd4c414a64c..de1d79fcb98 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -121,6 +121,16 @@ Anything following the symbol 'mode-line-format-right-align' in right-aligned to is controlled by the new user option 'mode-line-right-align-edge'. +** Emacs Server and Client + +--- +*** 'server-eval-args-left' can be used to pop subsequent eval args +When '--eval' is passed to emacsclient and Emacs is evaluating each +argument, this variable is set to those which have not yet been +evaluated. It can be used to 'pop' arguments to prevent them from +being evaluated, which is useful when those arguments contain +arbitrary data. + \f * Editing Changes in Emacs 30.1 diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..5962fa1764c 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,7 +13,7 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --eval '(message-mailto (pop server-eval-args-left))' %u [Desktop Action new-instance] Name=New Instance diff --git a/lisp/server.el b/lisp/server.el index c3325e5a24c..3970d7a7e81 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -1189,6 +1189,7 @@ server-process-filter parent-id ; Window ID for XEmbed dontkill ; t if client should not be killed. commands + evalexprs dir use-current-frame frame-parameters ;parameters for newly created frame @@ -1319,8 +1320,7 @@ server-process-filter (let ((expr (pop args-left))) (if coding-system (setq expr (decode-coding-string expr coding-system))) - (push (lambda () (server-eval-and-print expr proc)) - commands) + (push expr evalexprs) (setq filepos nil))) ;; -env NAME=VALUE: An environment variable. @@ -1345,7 +1345,7 @@ server-process-filter ;; arguments, use an existing frame. (and nowait (not (eq tty-name 'window-system)) - (or files commands) + (or files commands evalexprs) (setq use-current-frame t)) (setq frame @@ -1394,7 +1394,7 @@ server-process-filter (let ((default-directory (if (and dir (file-directory-p dir)) dir default-directory))) - (server-execute proc files nowait commands + (server-execute proc files nowait commands evalexprs dontkill frame tty-name))))) (when (or frame files) @@ -1404,22 +1404,35 @@ server-process-filter ;; condition-case (t (server-return-error proc err)))) -(defun server-execute (proc files nowait commands dontkill frame tty-name) +(defvar server-eval-args-left nil + "List of eval args not yet processed. + +Adding or removing strings from this variable while the Emacs +server is processing a series of eval requests will affect what +Emacs evaluates. + +See also `argv' for a similar variable which works for +invocations of \"emacs\".") + +(defun server-execute (proc files nowait commands evalexprs dontkill frame tty-name) ;; This is run from timers and process-filters, i.e. "asynchronously". ;; But w.r.t the user, this is not really asynchronous since the timer ;; is run after 0s and the process-filter is run in response to the ;; user running `emacsclient'. So it is OK to override the - ;; inhibit-quit flag, which is good since `commands' (as well as + ;; inhibit-quit flag, which is good since `evalexprs' (as well as ;; find-file-noselect via the major-mode) can run arbitrary code, ;; including code that needs to wait. (with-local-quit (condition-case err (let ((buffers (server-visit-files files proc nowait))) (mapc 'funcall (nreverse commands)) + (let ((server-eval-args-left (nreverse evalexprs))) + (while server-eval-args-left + (server-eval-and-print (pop server-eval-args-left) proc))) ;; If we were told only to open a new client, obey ;; `initial-buffer-choice' if it specifies a file ;; or a function. - (unless (or files commands) + (unless (or files commands evalexprs) (let ((buf (cond ((stringp initial-buffer-choice) (find-file-noselect initial-buffer-choice)) diff --git a/lisp/startup.el b/lisp/startup.el index 7f601668369..f2c84751211 100644 --- a/lisp/startup.el +++ b/lisp/startup.el @@ -120,7 +120,10 @@ command-switch-alist "List of command-line args not yet processed. This is a convenience alias, so that one can write (pop argv) inside of --eval command line arguments in order to access -following arguments.")) +following arguments. + +See also `server-eval-args-left' for a similar variable which +works for invocations of \"emacsclient --eval\".")) (internal-make-var-non-special 'argv) (defvar command-line-args-left nil -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-24 14:20 ` sbaugh @ 2023-10-21 15:20 ` sbaugh 2023-10-22 5:27 ` Eli Zaretskii 2023-10-22 5:39 ` Jim Porter 0 siblings, 2 replies; 59+ messages in thread From: sbaugh @ 2023-10-21 15:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, jporterbugs, sbaugh Any remaining concerns about this patch? It would be nice to install it. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-21 15:20 ` sbaugh @ 2023-10-22 5:27 ` Eli Zaretskii 2023-10-22 14:15 ` sbaugh 2023-10-22 5:39 ` Jim Porter 1 sibling, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-10-22 5:27 UTC (permalink / raw) To: sbaugh; +Cc: 65902, jporterbugs, sbaugh > From: sbaugh@catern.com > Date: Sat, 21 Oct 2023 15:20:13 +0000 (UTC) > Cc: jporterbugs@gmail.com, sbaugh@janestreet.com, 65902@debbugs.gnu.org > > > Any remaining concerns about this patch? It would be nice to install > it. It no longer applies, so please rebase and repost. TIA. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-22 5:27 ` Eli Zaretskii @ 2023-10-22 14:15 ` sbaugh 2023-10-22 16:09 ` Andreas Schwab 0 siblings, 1 reply; 59+ messages in thread From: sbaugh @ 2023-10-22 14:15 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, sbaugh, jporterbugs [-- Attachment #1: Type: text/plain, Size: 346 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: sbaugh@catern.com >> Date: Sat, 21 Oct 2023 15:20:13 +0000 (UTC) >> Cc: jporterbugs@gmail.com, sbaugh@janestreet.com, 65902@debbugs.gnu.org >> >> >> Any remaining concerns about this patch? It would be nice to install >> it. > > It no longer applies, so please rebase and repost. TIA. Rebased [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-server-eval-args-left-to-server.el.patch --] [-- Type: text/x-patch, Size: 8438 bytes --] From bd49b918e57363593660f605c3e0efc3d0155c2b Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Thu, 21 Sep 2023 21:35:50 -0400 Subject: [PATCH] Add server-eval-args-left to server.el Passing arbitrary arguments to functions through emacsclient --eval requires complicated escaping to avoid them being parsed as Lisp (as seen in emacsclient-mail.desktop before this change). This new variable server-eval-args-left allows access to the arguments before they are parsed as Lisp. By removing arguments from the variable before they're parsed, a snippet of Lisp can consume arguments, as in emacsclient-mail.desktop. org-protocol might be able to use this as well, which might allow it to drop its current advice on server-visit-files. * etc/emacsclient-mail.desktop: Use server-eval-args-left. (bug#65902) * lisp/server.el (server-eval-args-left): Add. (server-process-filter, server-execute): Make -eval arguments available through server-eval-args-left. * lisp/startup.el (argv): Mention server-eval-args-left in docstring. * etc/NEWS: Announce server-eval-args-left. * doc/emacs/misc.texi (emacsclient Options): Document server-eval-args-left. --- doc/emacs/misc.texi | 9 +++++++++ etc/NEWS | 10 ++++++++++ etc/emacsclient-mail.desktop | 7 ++----- lisp/server.el | 27 ++++++++++++++++++++------- lisp/startup.el | 5 ++++- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi index 7a88b7ef5e0..1ce1713b01c 100644 --- a/doc/emacs/misc.texi +++ b/doc/emacs/misc.texi @@ -2078,6 +2078,15 @@ emacsclient Options @command{emacsclient} are interpreted as a list of expressions to evaluate, @emph{not} as a list of files to visit. +@vindex server-eval-args-left +If you have arbitrary data which you want to provide as input to one +of your expressions, you can pass the data as another argument to +@command{emacsclient} and use @var{server-eval-args-left} in the +expression to access the data. Be careful to have your expression +remove the data from @var{server-eval-args-left} regardless of whether +your code succeeds, such as by using @code{pop}, otherwise Emacs will +attempt to evaluate the data as a Lisp expression. + @item -f @var{server-file} @itemx --server-file=@var{server-file} Specify a server file (@pxref{TCP Emacs server}) for connecting to an diff --git a/etc/NEWS b/etc/NEWS index cd4c414a64c..de1d79fcb98 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -121,6 +121,16 @@ Anything following the symbol 'mode-line-format-right-align' in right-aligned to is controlled by the new user option 'mode-line-right-align-edge'. +** Emacs Server and Client + +--- +*** 'server-eval-args-left' can be used to pop subsequent eval args +When '--eval' is passed to emacsclient and Emacs is evaluating each +argument, this variable is set to those which have not yet been +evaluated. It can be used to 'pop' arguments to prevent them from +being evaluated, which is useful when those arguments contain +arbitrary data. + \f * Editing Changes in Emacs 30.1 diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..5962fa1764c 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,7 +13,7 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --eval '(message-mailto (pop server-eval-args-left))' %u [Desktop Action new-instance] Name=New Instance diff --git a/lisp/server.el b/lisp/server.el index c3325e5a24c..3970d7a7e81 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -1189,6 +1189,7 @@ server-process-filter parent-id ; Window ID for XEmbed dontkill ; t if client should not be killed. commands + evalexprs dir use-current-frame frame-parameters ;parameters for newly created frame @@ -1319,8 +1320,7 @@ server-process-filter (let ((expr (pop args-left))) (if coding-system (setq expr (decode-coding-string expr coding-system))) - (push (lambda () (server-eval-and-print expr proc)) - commands) + (push expr evalexprs) (setq filepos nil))) ;; -env NAME=VALUE: An environment variable. @@ -1345,7 +1345,7 @@ server-process-filter ;; arguments, use an existing frame. (and nowait (not (eq tty-name 'window-system)) - (or files commands) + (or files commands evalexprs) (setq use-current-frame t)) (setq frame @@ -1394,7 +1394,7 @@ server-process-filter (let ((default-directory (if (and dir (file-directory-p dir)) dir default-directory))) - (server-execute proc files nowait commands + (server-execute proc files nowait commands evalexprs dontkill frame tty-name))))) (when (or frame files) @@ -1404,22 +1404,35 @@ server-process-filter ;; condition-case (t (server-return-error proc err)))) -(defun server-execute (proc files nowait commands dontkill frame tty-name) +(defvar server-eval-args-left nil + "List of eval args not yet processed. + +Adding or removing strings from this variable while the Emacs +server is processing a series of eval requests will affect what +Emacs evaluates. + +See also `argv' for a similar variable which works for +invocations of \"emacs\".") + +(defun server-execute (proc files nowait commands evalexprs dontkill frame tty-name) ;; This is run from timers and process-filters, i.e. "asynchronously". ;; But w.r.t the user, this is not really asynchronous since the timer ;; is run after 0s and the process-filter is run in response to the ;; user running `emacsclient'. So it is OK to override the - ;; inhibit-quit flag, which is good since `commands' (as well as + ;; inhibit-quit flag, which is good since `evalexprs' (as well as ;; find-file-noselect via the major-mode) can run arbitrary code, ;; including code that needs to wait. (with-local-quit (condition-case err (let ((buffers (server-visit-files files proc nowait))) (mapc 'funcall (nreverse commands)) + (let ((server-eval-args-left (nreverse evalexprs))) + (while server-eval-args-left + (server-eval-and-print (pop server-eval-args-left) proc))) ;; If we were told only to open a new client, obey ;; `initial-buffer-choice' if it specifies a file ;; or a function. - (unless (or files commands) + (unless (or files commands evalexprs) (let ((buf (cond ((stringp initial-buffer-choice) (find-file-noselect initial-buffer-choice)) diff --git a/lisp/startup.el b/lisp/startup.el index 7f601668369..f2c84751211 100644 --- a/lisp/startup.el +++ b/lisp/startup.el @@ -120,7 +120,10 @@ command-switch-alist "List of command-line args not yet processed. This is a convenience alias, so that one can write (pop argv) inside of --eval command line arguments in order to access -following arguments.")) +following arguments. + +See also `server-eval-args-left' for a similar variable which +works for invocations of \"emacsclient --eval\".")) (internal-make-var-non-special 'argv) (defvar command-line-args-left nil -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-22 14:15 ` sbaugh @ 2023-10-22 16:09 ` Andreas Schwab 2023-10-22 19:53 ` sbaugh 0 siblings, 1 reply; 59+ messages in thread From: Andreas Schwab @ 2023-10-22 16:09 UTC (permalink / raw) To: sbaugh; +Cc: jporterbugs, 65902, Eli Zaretskii, sbaugh On Okt 22 2023, sbaugh@catern.com wrote: > +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u This line is not valid according to the specs. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-22 16:09 ` Andreas Schwab @ 2023-10-22 19:53 ` sbaugh 2023-10-23 16:38 ` Andreas Schwab 2023-10-23 16:52 ` Jim Porter 0 siblings, 2 replies; 59+ messages in thread From: sbaugh @ 2023-10-22 19:53 UTC (permalink / raw) To: Andreas Schwab; +Cc: jporterbugs, Eli Zaretskii, sbaugh, 65902 Andreas Schwab <schwab@linux-m68k.org> writes: > On Okt 22 2023, sbaugh@catern.com wrote: > >> +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u > > This line is not valid according to the specs. Is that because of the single quotes surrounding spaces? Are quoted strings not permitted? Would the following be valid: Exec=emacsclient --alternate-editor= --eval '(message-mailto)' %u or perhaps Exec=emacsclient --alternate-editor= --eval (message-mailto) %u That's easily achievable. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-22 19:53 ` sbaugh @ 2023-10-23 16:38 ` Andreas Schwab 2023-10-23 16:52 ` Jim Porter 1 sibling, 0 replies; 59+ messages in thread From: Andreas Schwab @ 2023-10-23 16:38 UTC (permalink / raw) To: sbaugh; +Cc: jporterbugs, Eli Zaretskii, sbaugh, 65902 All of them contain unquoted occurences of reserved characters. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-22 19:53 ` sbaugh 2023-10-23 16:38 ` Andreas Schwab @ 2023-10-23 16:52 ` Jim Porter 2023-10-24 16:27 ` sbaugh 1 sibling, 1 reply; 59+ messages in thread From: Jim Porter @ 2023-10-23 16:52 UTC (permalink / raw) To: sbaugh, Andreas Schwab; +Cc: sbaugh, Eli Zaretskii, 65902 On 10/22/2023 12:53 PM, sbaugh@catern.com wrote: > Andreas Schwab <schwab@linux-m68k.org> writes: > >> On Okt 22 2023, sbaugh@catern.com wrote: >> >>> +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u >> >> This line is not valid according to the specs. > > Is that because of the single quotes surrounding spaces? Are quoted > strings not permitted? According to the spec[1], you should use double quotes instead: > Quoting must be done by enclosing the argument between double quotes and escaping the double quote character, backtick character ("`"), dollar sign ("$") and backslash character ("\") by preceding it with an additional backslash character. Otherwise, I think what you have above is ok. [1] https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s07.html ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-23 16:52 ` Jim Porter @ 2023-10-24 16:27 ` sbaugh 2023-10-29 12:20 ` Eli Zaretskii 0 siblings, 1 reply; 59+ messages in thread From: sbaugh @ 2023-10-24 16:27 UTC (permalink / raw) To: Jim Porter; +Cc: sbaugh, Eli Zaretskii, Andreas Schwab, 65902 [-- Attachment #1: Type: text/plain, Size: 940 bytes --] Jim Porter <jporterbugs@gmail.com> writes: > On 10/22/2023 12:53 PM, sbaugh@catern.com wrote: >> Andreas Schwab <schwab@linux-m68k.org> writes: >> >>> On Okt 22 2023, sbaugh@catern.com wrote: >>> >>>> +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u >>> >>> This line is not valid according to the specs. >> Is that because of the single quotes surrounding spaces? Are quoted >> strings not permitted? > > According to the spec[1], you should use double quotes instead: > >> Quoting must be done by enclosing the argument between double quotes and escaping the double quote character, backtick character ("`"), dollar sign ("$") and backslash character ("\") by preceding it with an additional backslash character. > > Otherwise, I think what you have above is ok. > > [1] > https://specifications.freedesktop.org/desktop-entry-spec/latest/ar01s07.html Thanks. OK, spec-compliant version: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-server-eval-args-left-to-server.el.patch --] [-- Type: text/x-patch, Size: 8466 bytes --] From 59d780f3297134d97cd39a8a4b0fb722d3d42765 Mon Sep 17 00:00:00 2001 From: Spencer Baugh <sbaugh@catern.com> Date: Thu, 21 Sep 2023 21:35:50 -0400 Subject: [PATCH] Add server-eval-args-left to server.el Passing arbitrary arguments to functions through emacsclient --eval requires complicated escaping to avoid them being parsed as Lisp (as seen in emacsclient-mail.desktop before this change). This new variable server-eval-args-left allows access to the arguments before they are parsed as Lisp. By removing arguments from the variable before they're parsed, a snippet of Lisp can consume arguments, as in emacsclient-mail.desktop. org-protocol might be able to use this as well, which might allow it to drop its current advice on server-visit-files. * etc/emacsclient-mail.desktop: Use server-eval-args-left. (bug#65902) * lisp/server.el (server-eval-args-left): Add. (server-process-filter, server-execute): Make -eval arguments available through server-eval-args-left. * lisp/startup.el (argv): Mention server-eval-args-left in docstring. * etc/NEWS: Announce server-eval-args-left. * doc/emacs/misc.texi (emacsclient Options): Document server-eval-args-left. --- doc/emacs/misc.texi | 9 +++++++++ etc/NEWS | 10 ++++++++++ etc/emacsclient-mail.desktop | 7 ++----- lisp/server.el | 27 ++++++++++++++++++++------- lisp/startup.el | 5 ++++- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi index d7168fa1ca0..9c7c5dcd5da 100644 --- a/doc/emacs/misc.texi +++ b/doc/emacs/misc.texi @@ -2078,6 +2078,15 @@ emacsclient Options @command{emacsclient} are interpreted as a list of expressions to evaluate, @emph{not} as a list of files to visit. +@vindex server-eval-args-left +If you have arbitrary data which you want to provide as input to one +of your expressions, you can pass the data as another argument to +@command{emacsclient} and use @var{server-eval-args-left} in the +expression to access the data. Be careful to have your expression +remove the data from @var{server-eval-args-left} regardless of whether +your code succeeds, such as by using @code{pop}, otherwise Emacs will +attempt to evaluate the data as a Lisp expression. + @item -f @var{server-file} @itemx --server-file=@var{server-file} Specify a server file (@pxref{TCP Emacs server}) for connecting to an diff --git a/etc/NEWS b/etc/NEWS index e6c47660522..75da416c5f3 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -212,6 +212,16 @@ to enter the file you want to modify. It can be used to customize the look of the appointment notification displayed on the mode line when 'appt-display-mode-line' is non-nil. +** Emacs Server and Client + +--- +*** 'server-eval-args-left' can be used to pop subsequent eval args +When '--eval' is passed to emacsclient and Emacs is evaluating each +argument, this variable is set to those which have not yet been +evaluated. It can be used to 'pop' arguments to prevent them from +being evaluated, which is useful when those arguments contain +arbitrary data. + \f * Editing Changes in Emacs 30.1 diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop index 0a2420ddead..4f7f00ebefd 100644 --- a/etc/emacsclient-mail.desktop +++ b/etc/emacsclient-mail.desktop @@ -1,10 +1,7 @@ [Desktop Entry] Categories=Network;Email; Comment=GNU Emacs is an extensible, customizable text editor - and more -# We want to pass the following commands to the shell wrapper: -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --eval "(message-mailto (pop server-eval-args-left))" %u Icon=emacs Name=Emacs (Mail, Client) MimeType=x-scheme-handler/mailto; @@ -16,7 +13,7 @@ Actions=new-window;new-instance; [Desktop Action new-window] Name=New Window -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u +Exec=emacsclient --alternate-editor= --create-frame --eval "(message-mailto (pop server-eval-args-left))" %u [Desktop Action new-instance] Name=New Instance diff --git a/lisp/server.el b/lisp/server.el index ce68e9aebc9..a2671165bfc 100644 --- a/lisp/server.el +++ b/lisp/server.el @@ -1199,6 +1199,7 @@ server-process-filter parent-id ; Window ID for XEmbed dontkill ; t if client should not be killed. commands + evalexprs dir use-current-frame frame-parameters ;parameters for newly created frame @@ -1332,8 +1333,7 @@ server-process-filter (let ((expr (pop args-left))) (if coding-system (setq expr (decode-coding-string expr coding-system))) - (push (lambda () (server-eval-and-print expr proc)) - commands) + (push expr evalexprs) (setq filepos nil))) ;; -env NAME=VALUE: An environment variable. @@ -1358,7 +1358,7 @@ server-process-filter ;; arguments, use an existing frame. (and nowait (not (eq tty-name 'window-system)) - (or files commands) + (or files commands evalexprs) (setq use-current-frame t)) (setq frame @@ -1407,7 +1407,7 @@ server-process-filter (let ((default-directory (if (and dir (file-directory-p dir)) dir default-directory))) - (server-execute proc files nowait commands + (server-execute proc files nowait commands evalexprs dontkill frame tty-name))))) (when (or frame files) @@ -1417,22 +1417,35 @@ server-process-filter ;; condition-case (t (server-return-error proc err)))) -(defun server-execute (proc files nowait commands dontkill frame tty-name) +(defvar server-eval-args-left nil + "List of eval args not yet processed. + +Adding or removing strings from this variable while the Emacs +server is processing a series of eval requests will affect what +Emacs evaluates. + +See also `argv' for a similar variable which works for +invocations of \"emacs\".") + +(defun server-execute (proc files nowait commands evalexprs dontkill frame tty-name) ;; This is run from timers and process-filters, i.e. "asynchronously". ;; But w.r.t the user, this is not really asynchronous since the timer ;; is run after 0s and the process-filter is run in response to the ;; user running `emacsclient'. So it is OK to override the - ;; inhibit-quit flag, which is good since `commands' (as well as + ;; inhibit-quit flag, which is good since `evalexprs' (as well as ;; find-file-noselect via the major-mode) can run arbitrary code, ;; including code that needs to wait. (with-local-quit (condition-case err (let ((buffers (server-visit-files files proc nowait))) (mapc 'funcall (nreverse commands)) + (let ((server-eval-args-left (nreverse evalexprs))) + (while server-eval-args-left + (server-eval-and-print (pop server-eval-args-left) proc))) ;; If we were told only to open a new client, obey ;; `initial-buffer-choice' if it specifies a file ;; or a function. - (unless (or files commands) + (unless (or files commands evalexprs) (let ((buf (cond ((stringp initial-buffer-choice) (find-file-noselect initial-buffer-choice)) diff --git a/lisp/startup.el b/lisp/startup.el index 6329e3ea8d0..37843eab176 100644 --- a/lisp/startup.el +++ b/lisp/startup.el @@ -120,7 +120,10 @@ command-switch-alist "List of command-line args not yet processed. This is a convenience alias, so that one can write (pop argv) inside of --eval command line arguments in order to access -following arguments.")) +following arguments. + +See also `server-eval-args-left' for a similar variable which +works for invocations of \"emacsclient --eval\".")) (internal-make-var-non-special 'argv) (defvar command-line-args-left nil -- 2.41.0 ^ permalink raw reply related [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-24 16:27 ` sbaugh @ 2023-10-29 12:20 ` Eli Zaretskii 0 siblings, 0 replies; 59+ messages in thread From: Eli Zaretskii @ 2023-10-29 12:20 UTC (permalink / raw) To: sbaugh; +Cc: jporterbugs, 65902-done, schwab, sbaugh > From: sbaugh@catern.com > Date: Tue, 24 Oct 2023 16:27:33 +0000 (UTC) > Cc: Andreas Schwab <schwab@linux-m68k.org>, sbaugh@janestreet.com, > Eli Zaretskii <eliz@gnu.org>, 65902@debbugs.gnu.org > > Thanks. OK, spec-compliant version: Thanks, installed on master, and closing the bug. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-10-21 15:20 ` sbaugh 2023-10-22 5:27 ` Eli Zaretskii @ 2023-10-22 5:39 ` Jim Porter 1 sibling, 0 replies; 59+ messages in thread From: Jim Porter @ 2023-10-22 5:39 UTC (permalink / raw) To: sbaugh, Eli Zaretskii; +Cc: 65902, sbaugh On 10/21/2023 8:20 AM, sbaugh@catern.com wrote: > > Any remaining concerns about this patch? It would be nice to install > it. From my perspective, I'm happy with this change. Thanks for all the effort on seeing this through. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 1:36 ` sbaugh 2023-09-22 6:36 ` Eli Zaretskii @ 2023-09-22 7:05 ` Stefan Kangas 2023-09-22 7:14 ` Eli Zaretskii 1 sibling, 1 reply; 59+ messages in thread From: Stefan Kangas @ 2023-09-22 7:05 UTC (permalink / raw) To: sbaugh, Eli Zaretskii; +Cc: jporterbugs, 65902, Spencer Baugh sbaugh@catern.com writes: > diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop > index 0a2420ddead..5962fa1764c 100644 > --- a/etc/emacsclient-mail.desktop > +++ b/etc/emacsclient-mail.desktop > @@ -1,10 +1,7 @@ > [Desktop Entry] > Categories=Network;Email; > Comment=GNU Emacs is an extensible, customizable text editor - and more > -# We want to pass the following commands to the shell wrapper: > -# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")" > -# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'. > -Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u > +Exec=emacsclient --alternate-editor= --eval '(message-mailto (pop server-eval-args-left))' %u As Spencer pointed out upthread, the mailto: links come from untrusted sources (e.g. websites). Escaping is infamous for being hard to get right, and for that reason is a popular attack vector among bad actors. I think it would be good if we could reduce the amount of stuff we have to remember escaping here, or even better if we didn't need to escape anything at all. It's analogous to the case `shell-command-to-string' (which uses a shell) vs `call-process' (which doesn't). To my mind, this speaks in favor of some type of change in this direction. My two cents. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 7:05 ` Stefan Kangas @ 2023-09-22 7:14 ` Eli Zaretskii 2023-09-22 9:29 ` Andreas Schwab 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-22 7:14 UTC (permalink / raw) To: Stefan Kangas; +Cc: jporterbugs, sbaugh, sbaugh, 65902 > From: Stefan Kangas <stefankangas@gmail.com> > Date: Fri, 22 Sep 2023 00:05:20 -0700 > Cc: 65902@debbugs.gnu.org, Spencer Baugh <sbaugh@janestreet.com>, jporterbugs@gmail.com > > I think it would be good if we could reduce the amount of stuff we have > to remember escaping here, or even better if we didn't need to escape > anything at all. We cannot avoid some escaping because the command is invoked through "sh -c". This is not our decision, this is the decision of freedesktop.org, and we have no control on what they decide, even if we think the decision is not the best one. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 7:14 ` Eli Zaretskii @ 2023-09-22 9:29 ` Andreas Schwab 2023-09-22 11:32 ` Eli Zaretskii 0 siblings, 1 reply; 59+ messages in thread From: Andreas Schwab @ 2023-09-22 9:29 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, jporterbugs, Stefan Kangas, sbaugh, sbaugh On Sep 22 2023, Eli Zaretskii wrote: > We cannot avoid some escaping because the command is invoked through > "sh -c". This is not our decision, this is the decision of > freedesktop.org, That's not true. The Exec line is _not_ passed to the shell, but is processed by a specialized parser and then executed directly. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 9:29 ` Andreas Schwab @ 2023-09-22 11:32 ` Eli Zaretskii 2023-09-22 12:37 ` Andreas Schwab 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-22 11:32 UTC (permalink / raw) To: Andreas Schwab; +Cc: 65902, jporterbugs, stefankangas, sbaugh, sbaugh > From: Andreas Schwab <schwab@linux-m68k.org> > Cc: Stefan Kangas <stefankangas@gmail.com>, jporterbugs@gmail.com, > sbaugh@catern.com, sbaugh@janestreet.com, 65902@debbugs.gnu.org > Date: Fri, 22 Sep 2023 11:29:06 +0200 > > On Sep 22 2023, Eli Zaretskii wrote: > > > We cannot avoid some escaping because the command is invoked through > > "sh -c". This is not our decision, this is the decision of > > freedesktop.org, > > That's not true. The Exec line is _not_ passed to the shell, but is > processed by a specialized parser and then executed directly. Then why is "sh -c" being used? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 11:32 ` Eli Zaretskii @ 2023-09-22 12:37 ` Andreas Schwab 2023-09-22 12:56 ` Eli Zaretskii 0 siblings, 1 reply; 59+ messages in thread From: Andreas Schwab @ 2023-09-22 12:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, jporterbugs, stefankangas, sbaugh, sbaugh On Sep 22 2023, Eli Zaretskii wrote: > Then why is "sh -c" being used? This is self-inflicted pain, because emacsclient does not support passing arbitrary arguments to emacs. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 12:37 ` Andreas Schwab @ 2023-09-22 12:56 ` Eli Zaretskii 2023-09-22 13:23 ` Andreas Schwab 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-22 12:56 UTC (permalink / raw) To: Andreas Schwab; +Cc: 65902, jporterbugs, stefankangas, sbaugh, sbaugh > From: Andreas Schwab <schwab@linux-m68k.org> > Cc: stefankangas@gmail.com, jporterbugs@gmail.com, sbaugh@catern.com, > sbaugh@janestreet.com, 65902@debbugs.gnu.org > Date: Fri, 22 Sep 2023 14:37:53 +0200 > > On Sep 22 2023, Eli Zaretskii wrote: > > > Then why is "sh -c" being used? > > This is self-inflicted pain, because emacsclient does not support > passing arbitrary arguments to emacs. What do you mean by "arbitrary arguments"? what kind of arguments need to be passed here that are not currently supported? ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 12:56 ` Eli Zaretskii @ 2023-09-22 13:23 ` Andreas Schwab 2023-09-22 14:51 ` Eli Zaretskii 0 siblings, 1 reply; 59+ messages in thread From: Andreas Schwab @ 2023-09-22 13:23 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, jporterbugs, stefankangas, sbaugh, sbaugh On Sep 22 2023, Eli Zaretskii wrote: > What do you mean by "arbitrary arguments"? what kind of arguments need > to be passed here that are not currently supported? %u -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 13:23 ` Andreas Schwab @ 2023-09-22 14:51 ` Eli Zaretskii 2023-09-22 14:52 ` Andreas Schwab 0 siblings, 1 reply; 59+ messages in thread From: Eli Zaretskii @ 2023-09-22 14:51 UTC (permalink / raw) To: Andreas Schwab; +Cc: 65902, jporterbugs, stefankangas, sbaugh, sbaugh > From: Andreas Schwab <schwab@linux-m68k.org> > Cc: stefankangas@gmail.com, jporterbugs@gmail.com, sbaugh@catern.com, > sbaugh@janestreet.com, 65902@debbugs.gnu.org > Date: Fri, 22 Sep 2023 15:23:51 +0200 > > On Sep 22 2023, Eli Zaretskii wrote: > > > What do you mean by "arbitrary arguments"? what kind of arguments need > > to be passed here that are not currently supported? > > %u This contradicts what I see in the *desktop files. They use shell commands and features. See, for example, emacsclient.desktop and emacsclient-mail.desktop. ^ permalink raw reply [flat|nested] 59+ messages in thread
* bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping 2023-09-22 14:51 ` Eli Zaretskii @ 2023-09-22 14:52 ` Andreas Schwab 0 siblings, 0 replies; 59+ messages in thread From: Andreas Schwab @ 2023-09-22 14:52 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 65902, jporterbugs, stefankangas, sbaugh, sbaugh On Sep 22 2023, Eli Zaretskii wrote: >> From: Andreas Schwab <schwab@linux-m68k.org> >> Cc: stefankangas@gmail.com, jporterbugs@gmail.com, sbaugh@catern.com, >> sbaugh@janestreet.com, 65902@debbugs.gnu.org >> Date: Fri, 22 Sep 2023 15:23:51 +0200 >> >> On Sep 22 2023, Eli Zaretskii wrote: >> >> > What do you mean by "arbitrary arguments"? what kind of arguments need >> > to be passed here that are not currently supported? >> >> %u > > This contradicts what I see in the *desktop files. They use shell > commands and features. That's the self-inflicted pain. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 59+ messages in thread
end of thread, other threads:[~2023-10-29 12:20 UTC | newest] Thread overview: 59+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-13 2:24 bug#65902: 29.0.92; emacsclient-mail.desktop fails due to complicated escaping sbaugh 2023-09-13 2:30 ` sbaugh 2023-09-13 3:46 ` Jim Porter 2023-09-13 8:00 ` Robert Pluim 2023-09-13 13:06 ` Eli Zaretskii 2023-09-13 14:22 ` Robert Pluim 2023-09-13 12:41 ` Eli Zaretskii 2023-09-13 12:57 ` sbaugh 2023-09-13 12:41 ` Eli Zaretskii 2023-09-13 13:01 ` sbaugh 2023-09-13 13:26 ` Eli Zaretskii 2023-09-16 13:30 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] <fe2cc764-86c6-4840-80b7-8f3a3778b374@email.android.com> 2023-09-13 14:50 ` Eli Zaretskii 2023-09-13 15:01 ` Andreas Schwab 2023-09-13 15:23 ` Spencer Baugh 2023-09-13 16:19 ` Jim Porter 2023-09-13 19:13 ` Eli Zaretskii 2023-09-13 19:33 ` Jim Porter 2023-09-13 20:00 ` Spencer Baugh 2023-09-13 20:16 ` Jim Porter 2023-09-14 5:10 ` Eli Zaretskii 2023-09-14 11:03 ` sbaugh 2023-09-14 11:18 ` sbaugh 2023-09-14 11:35 ` sbaugh 2023-09-14 13:36 ` Eli Zaretskii 2023-09-14 14:04 ` Spencer Baugh 2023-09-14 14:31 ` Eli Zaretskii 2023-09-14 19:16 ` Jim Porter 2023-09-15 5:33 ` Eli Zaretskii 2023-09-16 13:43 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-09-16 14:02 ` Eli Zaretskii 2023-09-16 15:54 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors [not found] <80d8aeb0-c9f1-410f-b83d-60f83ca5b3af@email.android.com> 2023-09-14 14:57 ` Eli Zaretskii 2023-09-14 15:10 ` Spencer Baugh 2023-09-15 6:29 ` Eli Zaretskii 2023-09-22 1:36 ` sbaugh 2023-09-22 6:36 ` Eli Zaretskii 2023-09-23 20:24 ` sbaugh 2023-09-24 5:18 ` Eli Zaretskii 2023-09-24 14:20 ` sbaugh 2023-10-21 15:20 ` sbaugh 2023-10-22 5:27 ` Eli Zaretskii 2023-10-22 14:15 ` sbaugh 2023-10-22 16:09 ` Andreas Schwab 2023-10-22 19:53 ` sbaugh 2023-10-23 16:38 ` Andreas Schwab 2023-10-23 16:52 ` Jim Porter 2023-10-24 16:27 ` sbaugh 2023-10-29 12:20 ` Eli Zaretskii 2023-10-22 5:39 ` Jim Porter 2023-09-22 7:05 ` Stefan Kangas 2023-09-22 7:14 ` Eli Zaretskii 2023-09-22 9:29 ` Andreas Schwab 2023-09-22 11:32 ` Eli Zaretskii 2023-09-22 12:37 ` Andreas Schwab 2023-09-22 12:56 ` Eli Zaretskii 2023-09-22 13:23 ` Andreas Schwab 2023-09-22 14:51 ` Eli Zaretskii 2023-09-22 14:52 ` Andreas Schwab
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).