From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?Q?Sebasti=C3=A1n_Mon=C3=ADa?= Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Use vtable for eww-bookmarks Date: Wed, 11 Dec 2024 23:15:13 -0500 Message-ID: <87ttb9k04u.fsf@sebasmonia.com> References: <87ldx0vufd.fsf@sebasmonia.com> <8fde5a67-f778-f0c8-bcdd-ece08c95d369@gmail.com> <87h67nm4su.fsf@sebasmonia.com> <28ba395f-5c52-1fc8-a99a-9bae461520d8@gmail.com> <36a57a48-76ab-4254-b3bc-af1fff2d9b98@app.fastmail.com> 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="25403"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: "Adam Porter" , emacs-devel@gnu.org, "Eli Zaretskii" To: "Jim Porter" Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Thu Dec 12 05:16:23 2024 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 1tLacg-0006SD-R9 for ged-emacs-devel@m.gmane-mx.org; Thu, 12 Dec 2024 05:16:23 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tLaby-0006fx-L8; Wed, 11 Dec 2024 23:15:38 -0500 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 1tLabp-0006fI-IS for emacs-devel@gnu.org; Wed, 11 Dec 2024 23:15:29 -0500 Original-Received: from fout-a7-smtp.messagingengine.com ([103.168.172.150]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tLabm-0007Nx-Oj; Wed, 11 Dec 2024 23:15:29 -0500 Original-Received: from phl-compute-12.internal (phl-compute-12.phl.internal [10.202.2.52]) by mailfout.phl.internal (Postfix) with ESMTP id 75E8113836F5; Wed, 11 Dec 2024 23:15:14 -0500 (EST) Original-Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-12.internal (MEProxy); Wed, 11 Dec 2024 23:15:14 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sebasmonia.com; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm1; t=1733976914; x= 1734063314; bh=ogtK627+ba70xCCONjB4/P+cce3RiLcl6+r+sZwEm0Q=; b=A PpbBV2pmZCfWnD1LZd+UyJRT9PXgEQI/E5LNcYd8bXyMSsnPI+VpCkh07VqUryyd sYNdnchZN4SWljXO6Zsr6uZ8Lo4Nlpu7g9RyH84OevjGGNaZjnbPfvZUs4sbt/QS YwbZ/zFSSCR3kvS1XH/r5YSEdCcUS1BVi6oP6/uhRpERhFaQGeDFL8rSBmlj5HnI EOismtt5SCn+Szf3YBInUkN5935/K4AK5gB6cylHX63OOtMPZC7rJkqGLRDxnJib ym3uLssyXWdCedvKUm+Kn9+cogRBLSStlU54smUZq7eOaFHNiuBLJouDLuT6htYC WV+H6x7KndFC1mVVbOjbQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1733976914; x=1734063314; bh=ogtK627+ba70xCCONjB4/P+cce3RiLcl6+r +sZwEm0Q=; b=k9FBL9xwbqO6O32tDFqXWa3jbY9nhC+LnUNLki24peSXW+4juin TkgShb+mSa73mrjvCrteaQppuGnefC/ffeeeLHfvx5btfOYClAtr0z7qeOJyouQe kCSI63hOAR5O4ijh8tijygIAAA4ndT9hzTvcylQSS3XTesnm+Kul5GvDTCZwSvON ymfpBABlPK1Psi0S+IoAf8OlSYqp8qrUaDjIRkPlOdBdsSwSsoL6Ef8fK4fsyyEK WYTsodByI6mj90ghveg/US2mGr+UF4rWyXi2Ww5OGFgIlCtp9lSLp7E9UzYe2vHv 296vg0zRjTaPXes4Vr5gfYI6nsFZ5lv8seQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrkeefgdduvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpefhvfevufgjfhffkfgfgggtsehmtderredtreejnecu hfhrohhmpefuvggsrghsthhijohnucfoohhnvogruceoshgvsggrshhtihgrnhesshgvsg grshhmohhnihgrrdgtohhmqeenucggtffrrghtthgvrhhnpeehkedvueffgfeluedvueet hfejteetudefffelheejgeeugffgteetfedvvdekteenucevlhhushhtvghrufhiiigvpe dtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehsvggsrghsthhirghnsehsvggsrghsmhho nhhirgdrtghomhdpnhgspghrtghpthhtohepgedpmhhouggvpehsmhhtphhouhhtpdhrtg hpthhtohepvghlihiisehgnhhurdhorhhgpdhrtghpthhtohepvghmrggtshdquggvvhgv lhesghhnuhdrohhrghdprhgtphhtthhopegruggrmhesrghlphhhrghprghprgdrnhgvth dprhgtphhtthhopehjphhorhhtvghrsghughhssehgmhgrihhlrdgtohhm X-ME-Proxy: Feedback-ID: iab2c46da:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 11 Dec 2024 23:15:13 -0500 (EST) In-Reply-To: <36a57a48-76ab-4254-b3bc-af1fff2d9b98@app.fastmail.com> (=?utf-8?Q?=22Sebasti=C3=A1n_Mon=C3=ADa=22's?= message of "Tue, 10 Dec 2024 14:16:01 -0500") Received-SPF: pass client-ip=103.168.172.150; envelope-from=sebastian@sebasmonia.com; helo=fout-a7-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 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_LOW=-0.7, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_PASS=-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:326392 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Sebasti=C3=A1n Mon=C3=ADa writes: > On Sun, Dec 8, 2024, at 11:52 PM, Jim Porter wrote: >> On 12/1/2024 2:12 PM, Sebasti=C3=A1n Mon=C3=ADa wrote: >> > Generalizing this feature can be challenging in that different types of >> > information could need very different handing. >>=20 >> I think in this case, I'll defer to you and Adam (and Eli) about the=20 >> best path forward here. The existing behavior is a bit odd, and since I= =20 >> don't use EWW's bookmarks, I don't have a good sense of the right way to= =20 >> improve things here. Or in other words, I don't have any objections to=20 >> the code, but I'm not confident that I know enough about this corner to= =20 >> do a properly thorough review. > > Will submit a patch ASAP to include a column "Order". > And then we don't need a "remove sort" binding. Hi all, Attached a patch with "Order" and validation for this sort before kill/yank. As always, open to feedback. @Adam, apparently reverting the vtable re-generates all objects, when using a function (as is our case here). So we can't use `vtable-goto-object' unless the change from eq to equal is applied first. Sad indeed. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Use-vtable-in-eww-list-bookmarks.patch Content-Description: eww-bookmarks >From 840e8039d5e4868c89413f60b08d26d65d8c3ce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebasti=C3=A1n=20Mon=C3=ADa?= Date: Sun, 1 Dec 2024 00:29:08 -0500 Subject: [PATCH] Use vtable in eww-list-bookmarks --- lisp/net/eww.el | 174 +++++++++++++++++++++++++++--------------------- 1 file changed, 98 insertions(+), 76 deletions(-) diff --git a/lisp/net/eww.el b/lisp/net/eww.el index 4d4d4d6beac..0a842907bae 100644 --- a/lisp/net/eww.el +++ b/lisp/net/eww.el @@ -2359,6 +2359,7 @@ eww-add-bookmark (plist-get eww-data :title)))) (defun eww-write-bookmarks () + "Write the bookmarks in `eww-bookmarks-directory'." (with-temp-file (expand-file-name "eww-bookmarks" eww-bookmarks-directory) (insert ";; Auto-generated file; don't edit -*- mode: lisp-data -*-\n") (let ((print-length nil) @@ -2378,115 +2379,136 @@ eww-read-bookmarks (user-error "No bookmarks are defined")))) ;;;###autoload -(defun eww-list-bookmarks () - "Display the bookmarks." +(defun eww-list-bookmarks (&optional build-only) + "Display the eww bookmarks. +Optional argument BUILD-ONLY, when non-nil, means to build the buffer +without popping it." (interactive) (eww-read-bookmarks t) - (pop-to-buffer "*eww bookmarks*") - (eww-bookmark-prepare)) - -(defun eww-bookmark-prepare () - (set-buffer (get-buffer-create "*eww bookmarks*")) - (eww-bookmark-mode) - (let* ((width (/ (window-width) 2)) - (format (format "%%-%ds %%s" width)) - (inhibit-read-only t) - start title) + (with-current-buffer (get-buffer-create "*eww bookmarks*") + (eww-bookmark-mode) + (eww--bookmark-prepare)) + (unless build-only + (pop-to-buffer "*eww bookmarks*"))) + +(defun eww--bookmark-prepare () + "Display a table with the list of eww bookmarks. +Will remove all buffer contents first." + (let ((inhibit-read-only t)) (erase-buffer) - (setq header-line-format (concat " " (format format "Title" "URL"))) - (dolist (bookmark eww-bookmarks) - (setq start (point) - title (plist-get bookmark :title)) - (when (> (length title) width) - (setq title (truncate-string-to-width title width))) - (insert (format format title (plist-get bookmark :url)) "\n") - (put-text-property start (1+ start) 'eww-bookmark bookmark)) - (goto-char (point-min)))) + (make-vtable + :columns '((:name "Order" :min-width 6) + (:name "Title" :min-width "25%" :max-width "50%") + (:name "URL")) + :objects-function #'eww--bookmark-format-data + ;; use fixed-font face + :face 'default + :sort-by '((0 . ascend)) + ))) + +(defun eww--bookmark-format-data () + "Format `eww-bookmarks' for use in a vtable. +The data is returned as a list (order title url bookmark), for use +in of `eww-bookmark-mode'. Order stars counting from 1." + (seq-map-indexed (lambda (bm index) + (list + (+ 1 index) + (plist-get bm :title) + (plist-get bm :url) + bm)) + eww-bookmarks)) (defvar eww-bookmark-kill-ring nil) +(defun eww--bookmark-check-order-sort () + "Signal a user error unless the bookmark vtable is sorted by asc order." + ;; vtables sort respecting the previous sort column. As long as + ;; "order" was last, the kill/yank operations will make sense, no + ;; matter what sort was used before. + (when-let* ((the-table (vtable-current-table)) + (last-sort-pair (car (last (vtable-sort-by the-table))))) + (unless (and (= 0 (car last-sort-pair)) + (eq 'ascend (cdr last-sort-pair))) + (user-error + "Can't kill/yank bookmarks unless the table is sorted by ascending Order")))) + (defun eww-bookmark-kill () "Kill the current bookmark." (interactive nil eww-bookmark-mode) - (let* ((start (line-beginning-position)) - (bookmark (get-text-property start 'eww-bookmark)) - (inhibit-read-only t)) - (unless bookmark + (eww--bookmark-check-order-sort) + (let ((bookmark-at-point (nth 3 (vtable-current-object))) + (position (point))) + (unless bookmark-at-point (user-error "No bookmark on the current line")) (forward-line 1) - (push (buffer-substring start (point)) eww-bookmark-kill-ring) - (delete-region start (point)) - (setq eww-bookmarks (delq bookmark eww-bookmarks)) - (eww-write-bookmarks))) + (push bookmark-at-point eww-bookmark-kill-ring) + (setq eww-bookmarks (delq bookmark-at-point eww-bookmarks)) + (eww-write-bookmarks) + (vtable-revert-command) + (goto-char position))) (defun eww-bookmark-yank () "Yank a previously killed bookmark to the current line." (interactive nil eww-bookmark-mode) + (eww--bookmark-check-order-sort) (unless eww-bookmark-kill-ring (user-error "No previously killed bookmark")) - (beginning-of-line) - (let ((inhibit-read-only t) - (start (point)) - bookmark) - (insert (pop eww-bookmark-kill-ring)) - (setq bookmark (get-text-property start 'eww-bookmark)) - (if (= start (point-min)) - (push bookmark eww-bookmarks) - (let ((line (count-lines start (point)))) - (setcdr (nthcdr (1- line) eww-bookmarks) - (cons bookmark (nthcdr line eww-bookmarks))))) - (eww-write-bookmarks))) + (let* ((bookmark-at-point (nth 3 (vtable-current-object))) + (index-bap (seq-position eww-bookmarks bookmark-at-point)) + (position (point))) + ;; TODO: a simpler way of doing this? + (setq eww-bookmarks (seq-concatenate + 'list + (seq-subseq eww-bookmarks 0 index-bap) + (list (pop eww-bookmark-kill-ring)) + (seq-subseq eww-bookmarks index-bap))) + (eww-write-bookmarks) + (vtable-revert-command) + (vtable-beginning-of-table) + (goto-char position))) (defun eww-bookmark-browse () "Browse the bookmark under point in eww." (interactive nil eww-bookmark-mode) - (let ((bookmark (get-text-property (line-beginning-position) 'eww-bookmark))) - (unless bookmark + (let ((bookmark-at-point (nth 3 (vtable-current-object)))) + (unless bookmark-at-point (user-error "No bookmark on the current line")) (quit-window) - (eww-browse-url (plist-get bookmark :url)))) + (eww-browse-url (plist-get bookmark-at-point :url)))) (defun eww-next-bookmark () "Go to the next bookmark in the list." (interactive nil eww-bookmark-mode) - (let ((first nil) - bookmark) + (let (fresh-buffer target-bookmark) (unless (get-buffer "*eww bookmarks*") - (setq first t) - (eww-read-bookmarks t) - (eww-bookmark-prepare)) + (setq fresh-buffer t) + (eww-list-bookmarks t)) (with-current-buffer "*eww bookmarks*" - (when (and (not first) - (not (eobp))) - (forward-line 1)) - (setq bookmark (get-text-property (line-beginning-position) - 'eww-bookmark)) - (unless bookmark - (user-error "No next bookmark"))) - (eww-browse-url (plist-get bookmark :url)))) + (unless fresh-buffer + (forward-line 1)) + (setq target-bookmark (nth 3 (vtable-current-object)))) + (unless target-bookmark + ;; usually because we moved past end of the table + (user-error "No next bookmark")) + (eww-browse-url (plist-get target-bookmark :url)))) (defun eww-previous-bookmark () "Go to the previous bookmark in the list." (interactive nil eww-bookmark-mode) - (let ((first nil) - bookmark) + (let (fresh-buffer target-bookmark) (unless (get-buffer "*eww bookmarks*") - (setq first t) - (eww-read-bookmarks t) - (eww-bookmark-prepare)) + (setq fresh-buffer t) + (eww-list-bookmarks t)) (with-current-buffer "*eww bookmarks*" - (if first - (goto-char (point-max)) - (beginning-of-line)) - ;; On the final line. - (when (eolp) - (forward-line -1)) - (if (bobp) - (user-error "No previous bookmark") - (forward-line -1)) - (setq bookmark (get-text-property (line-beginning-position) - 'eww-bookmark))) - (eww-browse-url (plist-get bookmark :url)))) + (when fresh-buffer + (vtable-end-of-table)) + ;; didn't move to a previous line, because we + ;; were already on the first one + (unless (= -1 (forward-line -1)) + (setq target-bookmark (nth 3 (vtable-current-object))))) + (unless target-bookmark + (user-error "No previous bookmark")) + (eww-browse-url (plist-get target-bookmark :url)))) (defun eww-bookmark-urls () "Get the URLs from the current list of bookmarks." @@ -2501,9 +2523,9 @@ eww-bookmark-mode-map :menu '("Eww Bookmark" ["Exit" quit-window t] ["Browse" eww-bookmark-browse - :active (get-text-property (line-beginning-position) 'eww-bookmark)] + :active (nth 3 (vtable-current-object))] ["Kill" eww-bookmark-kill - :active (get-text-property (line-beginning-position) 'eww-bookmark)] + :active (nth 3 (vtable-current-object))] ["Yank" eww-bookmark-yank :active eww-bookmark-kill-ring])) -- 2.47.0 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable --=20 Sebasti=C3=A1n Mon=C3=ADa https://site.sebasmonia.com/ --=-=-=--