From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: [ELPA] New package: dape Date: Fri, 13 Oct 2023 12:24:43 +0000 Message-ID: <875y3ag16c.fsf@posteo.net> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="38801"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Daniel Pettersson Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Oct 13 14:25:49 2023 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qrHEh-0009oq-GS for ged-emacs-devel@m.gmane-mx.org; Fri, 13 Oct 2023 14:25:47 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qrHDp-0000Hy-38; Fri, 13 Oct 2023 08:24:53 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qrHDm-0000HF-Ep for emacs-devel@gnu.org; Fri, 13 Oct 2023 08:24:50 -0400 Original-Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qrHDj-000423-5K for emacs-devel@gnu.org; Fri, 13 Oct 2023 08:24:50 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id EEE8E240103 for ; Fri, 13 Oct 2023 14:24:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1697199885; bh=HNqKhR6OSSLv/NrpothjIMBlw5dazXvQTBTvWVesEvg=; h=From:To:Cc:Subject:Autocrypt:Date:Message-ID:MIME-Version:From; b=UvEj01yXxQSHlXPvEuqSB6v8QdsrFB/U4CwUAvXzBP5kVxz36jfVUbRHNFCrQC0Qn nDANghGpUGYhOMlA/abLupznnbD93ybs2lSm3tUBYZIL37iJZx1nP8tuO/SrSJkD1T qzbe9ZwxoVxj+Fk284NAGMWXauaoj/tEWBIT6H9ucVaDujG8nn4n1bu+Yml12FsjBJ kxglplpmwC+CpZUNr1klPq/k9wXqCfeSfB/ZLm5zG1KikhBPJDND2HVTnfM5hwhRBw QgfbKTxSVMDRi6py7L7vb6TRlOQ4QDBthY1RRL8faWhaPh1z7n4OhBvPFeBgZcBiBw 0xtd22HXOEjag== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4S6QgJ2bvDz6txL; Fri, 13 Oct 2023 14:24:44 +0200 (CEST) In-Reply-To: (Daniel Pettersson's message of "Fri, 13 Oct 2023 12:35:13 +0200") Autocrypt: addr=philipk@posteo.net; keydata= mDMEZBBQQhYJKwYBBAHaRw8BAQdAHJuofBrfqFh12uQu0Yi7mrl525F28eTmwUDflFNmdui0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiWBBMWCAA+FiEEDg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwMFCQHhM4AFCwkI BwIGFQoJCAsCBBYCAwECHgECF4AACgkQ8xYDWXahwulikAEA77hloUiSrXgFkUVJhlKBpLCHUjA0 mWZ9j9w5d08+jVwBAK6c4iGP7j+/PhbkxaEKa4V3MzIl7zJkcNNjHCXmvFcEuDgEZBBQQhIKKwYB BAGXVQEFAQEHQI5NLiLRjZy3OfSt1dhCmFyn+fN/QKELUYQetiaoe+MMAwEIB4h+BBgWCAAmFiEE Dg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwwFCQHhM4AACgkQ8xYDWXahwukm+wEA8cml4JpK NeAu65rg+auKrPOP6TP/4YWRCTIvuYDm0joBALw98AMz7/qMHvSCeU/hw9PL6u6R2EScxtpKnWof z4oM Received-SPF: pass client-ip=185.67.36.66; envelope-from=philipk@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:311433 Archived-At: --=-=-= Content-Type: text/plain Daniel Pettersson writes: > This package integrates debug adapters within Emacs. > Alternative to the well known package dap-mode. Very interesting! > See repository for code and more information https://github.com/svaante/dape Here are a few comments and suggestions I found from a brief skim over the code: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable diff --git a/dape.el b/dape.el index 7ce7132..47a1b17 100644 --- a/dape.el +++ b/dape.el @@ -5,6 +5,7 @@ ;; License: GPL-3.0-or-later ;; Version: 0.1 ;; Homepage: https://github.com/svaante/dape +;; Package-Requires: ((emacs "29.1")) =20 ;; This file is not part of GNU Emacs. =20 @@ -55,7 +56,7 @@ (defcustom dape-configs nil "This variable holds the Dape configurations as an alist. In this alist, the car element serves as a symbol identifying each -configuration. Each configuration, in turn, is a property list (plist) +configuration. Each configuration, in turn, is a property list (plist) where keys can be symbols or keywords. =20 Symbol Keys (Used by Dape): @@ -72,13 +73,13 @@ Debug adapter connection in configuration: - If only command is specified (without host and port), Dape will communicate with the debug adapter through stdin/stdout. - If both host and port are specified, Dape will connect to the - debug adapter. If `command is specified, Dape will wait until the + debug adapter. If `command is specified, Dape will wait until the command is initiated before it connects with host and port. =20 Keywords in configuration: Keywords are transmitted to the adapter during the initialize and - launch/attach requests. Refer to `json-serialize' for detailed - information on how Dape serializes these keyword elements. Dape + launch/attach requests. Refer to `json-serialize' for detailed + information on how Dape serializes these keyword elements. Dape uses nil as false. =20 Functions and symbols in configuration: @@ -160,10 +161,10 @@ The hook is run with one argument, the compilation bu= ffer." (defcustom dape--debug-on '(io info error std-server) "Types of logs should be printed to *dape-debug*." - :type '(list (const io :tag "dap IO") - (const info :tag "info logging") - (const error :tag "error logging") - (const std-server :tag "dap tcp server stdout"))) + :type '(list (const :tag "dap IO" io) + (const :tag "info logging" info) + (const :tag "error logging" error) + (const :tag "dap tcp server stdout" std-server))) ;;; Face =20 (defface dape-log-face @@ -323,8 +324,8 @@ The hook is run with one argument, the compilation buff= er." default-directory))) =20 (defun dape-find-file (&optional default) - (let* ((completion-ignored-extensions nil) - (default-directory (funcall dape-cwd-fn))) + (let ((completion-ignored-extensions nil) + (default-directory (funcall dape-cwd-fn))) (expand-file-name (read-file-name (if default (format "Program (default %s): " default) @@ -387,8 +388,8 @@ The hook is run with one argument, the compilation buff= er." (symbol-name type)) 'face 'match) " " - (apply 'format string objects))) - (newline))))) + (apply 'format string objects)) + "\n"))))) =20 (defun dape--live-process (&optional nowarn) (if (and dape--process @@ -684,7 +685,7 @@ The hook is run with one argument, the compilation buff= er." (defun dape--stack-trace (process thread cb) (cond ((or (plist-get (dape--current-thread) :stackFrames) - (not (numberp (plist-get thread :id)))) + (not (numberp (plist-get thread :id)))) ;any number? including fl= oats? (funcall cb process)) (t (dape-request process @@ -840,7 +841,7 @@ The hook is run with one argument, the compilation buff= er." =20 (cl-defmethod dape-handle-event (_process (_event (eql output)) body) (let ((category (plist-get body :category))) - (cond + (cond ;perhapse `pcase' would be nice ((equal category "stdout") (dape--repl-insert-text (plist-get body :output))) ((equal category "stderr") @@ -996,8 +997,7 @@ The hook is run with one argument, the compilation buff= er." (dape-request dape--process "restart" nil)) ((and dape--name dape--config) (dape dape--name dape--config)) - (t - (user-error "Unable to derive session to restart.")))) + ((user-error "Unable to derive session to restart")))) =20 (defun dape-kill () "Kill debug session." @@ -1164,7 +1164,7 @@ Executes launch `dape-configs' with :program as \"bin= \"." (seq-reduce (apply-partially 'apply 'plist-put) (seq-partition options 2) (copy-tree base-config)))) - (when (called-interactively-p) + (when (called-interactively-p 'interactive) (push (dape--config-to-string name base-config config) @@ -1232,13 +1232,9 @@ Watched symbols are displayed in *dape-info* buffer. ;;; Memory viewer =20 (defun dape--address-to-number (address) - (cond - ((and (> (length address) 2) - (equal "0x" (substring address 0 2))) - (string-to-number (string-trim-left address "0x0*") - 16)) - (t - (string-to-number address)))) + (if (string-match "\\`0x\\([[:alnum:]]+\\)" address) + (string-to-number (match-string 1 address) 16) + (string-to-number address))) =20 (defun dape-read-memory (memory-reference count) "Read COUNT bytes of memory at MEMORY-REFERENCE." @@ -1796,10 +1792,12 @@ See `dape-info' for more information." desktop-save-buffer nil tree-widget-image-enable nil)) =20 -(defun dape-info () +(defun dape-info (&optional select-buffer) "Create or select *dape-info* buffer. -Buffer contains debug session information." - (interactive) +Buffer contains debug session information. If the optional +argument SELECT-BUFFER is nil, or the command is not invoked +interactively, then the buffer is not displayed." ;perhaps rephrase this + (interactive (list t)) (let ((buffer (get-buffer-create "*dape-info*")) window) (with-current-buffer buffer @@ -1867,7 +1865,8 @@ Buffer contains debug session information." (setq window (display-buffer buffer '((display-buffer-in-side-window) . ((side . left))))) - (when (called-interactively-p) + + (when select-buffer (select-window window) (goto-char (point-min))))) =20 @@ -1882,32 +1881,31 @@ Buffer contains debug session information." (dape--repl-insert-text-guard (run-with-timer 0.1 nil 'dape--repl-insert-text msg)) (t - (progn - (setq dape--repl-insert-text-guard t) - (when-let ((buffer (get-buffer "*dape-repl*"))) - (with-current-buffer buffer - (save-excursion - (condition-case err - (progn - (goto-char (point-max)) - (comint-previous-prompt 0) - (forward-line -1) - (end-of-line) - (when-let (line (thing-at-point 'line)) - (when (eq (aref line 0) ?>) - (let ((inhibit-read-only t)) - (insert "\n")))) - (let ((inhibit-read-only t)) - (insert (propertize msg 'font-lock-face face)))) - (error - (setq dape--repl-insert-text-guard nil) - (signal (car err) (cdr err)))) - (setq dape--repl-insert-text-guard nil)))) - (unless (get-buffer-window "*dape-repl*") - (when (stringp msg) - (message (format "%s" - (string-trim msg "\\\n" "\\\n")) - 'face face))))))) + (setq dape--repl-insert-text-guard t) + (when-let ((buffer (get-buffer "*dape-repl*"))) + (with-current-buffer buffer + (save-excursion + (condition-case err + (progn + (goto-char (point-max)) + (comint-previous-prompt 0) + (forward-line -1) + (end-of-line) + (when-let (line (thing-at-point 'line)) + (when (eq (aref line 0) ?>) + (let ((inhibit-read-only t)) + (insert "\n")))) + (let ((inhibit-read-only t)) + (insert (propertize msg 'font-lock-face face)))) + (error + (setq dape--repl-insert-text-guard nil) + (signal (car err) (cdr err)))) + (setq dape--repl-insert-text-guard nil)))) + (unless (get-buffer-window "*dape-repl*") + (when (stringp msg) + (message (format "%s" + (string-trim msg "\\\n" "\\\n")) + 'face face)))))) =20 (defun dape--repl-input-sender (dummy-process input) (let (cmd) @@ -2050,7 +2048,7 @@ Buffer contains debug session information." :group 'dape :interactive nil (when dape-repl-mode - (user-error "`dape-repl-mode' all ready enabled.")) + (user-error "`dape-repl-mode' all ready enabled")) (setq-local dape-repl-mode t comint-prompt-read-only t comint-input-sender 'dape--repl-input-sender @@ -2099,7 +2097,7 @@ Empty input will rerun last command.\n\n\n" display-buffer-in-side-window) . ((side . bottom) (slot . -1))))) - (when (called-interactively-p) + (when (called-interactively-p) ;=E2=80=98called-interactively-p=E2= =80=99 called with 0 arguments, but requires 1! (select-window window))))) =20 @@ -2131,14 +2129,14 @@ Empty input will rerun last command.\n\n\n" (defun dape--config-from-string (str) (let (name read-config base-config) (when (string-empty-p str) - (user-error "Expected config name.")) + (user-error "Expected config name")) (setq name (read str) base-config (copy-tree (alist-get name dape-configs)) str (substring str (length (symbol-name name)))) (unless (string-empty-p str) (setq read-config (read (format "(%s)" str)))) (unless (plistp read-config) - (user-error "Unexpected config format, see `dape-configs'.")) + (user-error "Unexpected config format, see `dape-configs'")) (cl-loop for (key value) on read-config by 'cddr do (setq base-config (plist-put base-config key value))) (list name base-config))) @@ -2153,8 +2151,7 @@ Empty input will rerun last command.\n\n\n" (let ((config-diff (dape--config-diff pre-eval-config post-eval-config))) (concat (format "%s" name) - (when-let ((config-str (and config-diff - (prin1-to-string config-diff)))) + (and-let* ((config-diff) (config-str (prin1-to-string config-d= iff))) (format " %s" (substring config-str 1 @@ -2181,6 +2178,7 @@ Empty input will rerun last command.\n\n\n" ;;; Hover =20 (defun dape-hover-function (cb) + ;; Please consider addressing checkdoc issues like those raised here. (when-let ((symbol (thing-at-point 'symbol))) (dape--evaluate-expression (dape--live-process) (plist-get (dape--current-stack-frame) :id) @@ -2232,6 +2230,8 @@ Empty input will rerun last command.\n\n\n" ;;; Keymaps =20 +;; this raises the minimum version to 29.1, but you could take a look +;; at Compat to avoid this (defvar-keymap dape-global-map :doc "Keymap to repeat dape commands. Used in `repeat-mode'." "d" #'dape @@ -2251,6 +2251,8 @@ Empty input will rerun last command.\n\n\n" "w" #'dape-watch-dwim "q" #'dape-quit) =20 +;; Why are you mapping over the keymap, instead of just iterating over +;; the command you want to mark as repeatable? (map-keymap-internal (lambda (_ cmd) (unless (memq cmd '(dape dape-repl --=-=-= Content-Type: text/plain Also, sorry for bringing this up, but how married are you to the name? --=-=-=--