From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: James Thomas Newsgroups: gmane.emacs.devel Subject: Re: [ELPA] New package: tchanges Date: Mon, 23 Sep 2024 14:13:47 +0530 Message-ID: <86bk0evjy4.fsf@gmx.net> References: <86tte88sfl.fsf@gmx.net> <87o74f8uod.fsf@posteo.net> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="35919"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: emacs-devel@gnu.org To: Philip Kaludercic Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Sep 23 10:45:19 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 1sseh5-0009C3-F7 for ged-emacs-devel@m.gmane-mx.org; Mon, 23 Sep 2024 10:45:19 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ssegA-0007Fq-5O; Mon, 23 Sep 2024 04:44:22 -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 1sseg8-0007Fg-7L for emacs-devel@gnu.org; Mon, 23 Sep 2024 04:44:20 -0400 Original-Received: from mout.gmx.net ([212.227.17.21]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sseg0-0002LP-PO for emacs-devel@gnu.org; Mon, 23 Sep 2024 04:44:18 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmx.net; s=s31663417; t=1727081040; x=1727685840; i=jimjoe@gmx.net; bh=CDCBs9faEkcxdQf9ysxOIe4jkdxAxwidOu07OoeuAgI=; h=X-UI-Sender-Class:From:To:Cc:Subject:In-Reply-To:References:Date: Message-ID:MIME-Version:Content-Type:Content-Transfer-Encoding:cc: content-transfer-encoding:content-type:date:from:message-id: mime-version:reply-to:subject:to; b=X7AvCgko1TlAZ5hA38ZKrk+Fa3dLFqsreEgKFVqP3wGfGbTvM/bF6lHNvkFz/Kxl 1LOEcevGT36XvVZfkIknCmVTLl82Trt14dDrRvyewLHdi91/u8JSld3i70EN64lZW lWpg9aCHIZ/u0sM/9PwHK5LSHWc/r6kyjfLAlkqgfW6g1igB/LU96PJVpgBBUa1vh P5e2k0kTVidUVmpCwwVBU2Y46E3mupdQYisbw5h1Vxx9CCJ1lyCVRoD/hxrZqchlb 9FFwaIn0DbKs1tWEgINntYIdTxuiSKJesGzzE87p0iFdIm7IMln2v4J9K8mMFVNsl i3UV3ucAgYW26+iTnA== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Original-Received: from user-Inspiron-3493 ([117.206.139.11]) by mail.gmx.net (mrgmx104 [212.227.17.174]) with ESMTPSA (Nemesis) id 1MIwz4-1sdrbp2BTe-00KMdw; Mon, 23 Sep 2024 10:44:00 +0200 In-Reply-To: <87o74f8uod.fsf@posteo.net> (Philip Kaludercic's message of "Sun, 22 Sep 2024 17:28:18 +0000") X-Provags-ID: V03:K1:7Fn631W/+JUr7yZ8OqFLICrPnx9kag7Ptco5WNVJZ8p7vWsJ/af jyo5DasDrEroY0oPOVpR2UGj+eSTqIF1LtZYtch6PsbzKU2m7xy9RVWvTcgECj7HdkYg3F3 zrwQgOEIef9CTS2/yWJHoHCkbkhnwg8gXQCfUgk7ssPiR8O/1RwrwboTM9w7PCxTtofsxL+ 1oHcxBFbUXkl/X7LYIBlA== UI-OutboundReport: notjunk:1;M01:P0:HkW+8/UzDMs=;aC16I6/o1DEOPbL3KK9JbC8QIvR C0a5Gtf2NPFbF/ptHC4z7ZSUU1xTzdE0fFwJKoMrEA7PCZcByq6nSSOc+jL4XzBuIhqiFMlw2 Wvi27RQSkHhP1/i0reWwV9SH6EANt9VRVBsC72J/BouMxDQe/2NvpxdtLJotBIFwAlHfrWUjt YMmXL1RTbtLGxZQmwPJklQMwogqsZErygJ6+Y8SqENMe0HiutcUAOdu5GZrD3AHjXuDGWaDyb 4brR3sUwadR0pf5QAufo6co7qprslIx4Znvpng+agYR2iSWY3x2g9dycZLjzXs8LTa7jnqvls jWp8N8y6CRszAfGhMgzUO2DQwJtdMp0C5V/8vs8PMaRe1mcgHCK93+ZiVXHcEBt4yoYpCA4Vd 7mUaKwgKmIcnez/h+tNfnZRNqXq6gsEmqvVY6oTZdmccaBde2X6XcmBpLRlZp4SobGvF8+7A7 ok6uXQG7hzVQ9Yf1SXjDGUI1rS/OlCpUFPS3ZCZoGcWeR11maMhac04nVADeUHRaMiRX4nWQ+ CU1mP1Qz+E5KzvHt0MAaA7+dQVhtLeu2vVF/LFVieWSLA/2/MRJLzTv5iIN4xM2B5McQj9YE3 Q+HvV1wirfTrEO37RUjcqxzKLbEJV7lapgC1AJKcczN9RXISG4l1X/Y5BPLebB+NU4j9CWWSS YwOOmOTdBzG+ADXOyPzMj8SdcWPoMCYgP3Dq6781Lv84NGKhiQk75PIEm9jqeW3sXt+8HPBKq +qFQv9cDKnY6tcWgZCDWTmg9OcmGkd8N2xmNR+iNjE8PfXGkXy8Sark4kDPArXNhgAJC64uX Received-SPF: pass client-ip=212.227.17.21; envelope-from=jimjoe@gmx.net; helo=mout.gmx.net X-Spam_score_int: -36 X-Spam_score: -3.7 X-Spam_bar: --- X-Spam_report: (-3.7 / 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, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.897, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=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:323947 Archived-At: Hi, I've made all the changes except for the few ones noted below: Philip Kaludercic wrote: > James Thomas writes: > >> Hi, >> >> I propose to include this package that I wrote, to GNU ELPA. It's for >> collaborating with users of word-processing software, such as >> Libreoffice Writer, using the latter's 'track changes' feature. >> >> https://codeberg.org/quotuva/tchanges > > I haven't tested it, but here are a few quick comments: > > diff --git a/tchanges.el b/tchanges.el > index a3e6829..4bf6395 100644 > --- a/tchanges.el > +++ b/tchanges.el > @@ -5,6 +5,7 @@ > ;; Author: James Thomas > ;; Maintainer: James Thomas > ;; Keywords: wp, convenience, files > +;; Package-Requires: ((emacs "30")) Passed over in favour of removing the 'lessp' (below) as you suggested. > ;; URL: https://codeberg.org/quotuva/tchanges > ;; Version: 0.1 > > @@ -26,19 +27,24 @@ > (require 'bookmark) > > (defgroup tchanges nil > - "Collaborate with Office (docx) users using 'track changes'.") > + "Collaborate with Office (docx) users using 'track changes'." > + :group '???) ;this is missing! > > (defcustom tchanges-output-format > "org" > - "Default output format. See man page `pandoc(1)' for available option= s." > - :type '(string) > - :group 'tchanges) > + "Default output format. > +See man page `pandoc(1)' for available options." > + ;; By default, a user option is added to the last declared group, so > + ;; you can omit it here! > + :type 'string) ;Is this actually a string, or > + ;a symbol? Should you give a > + ;list of suggestions? Logically, no: because Pandoc's format code-names need not correspond to any Emacs major mode symbols, if any, for those formats. The only benefit to symbols I see is validation, but Pandoc could add new ones. > > (defcustom tchanges-pandoc-extra-args > nil > - "Extra args passed to pandoc. Value is a list of strings, which may b= e nil." > - :type '(repeat (string :tag "Argument")) > - :group 'tchanges) > + "Extra args passed to pandoc. > +Value is a list of strings, which may be nil." > + :type '(repeat :tag "Arguments" string)) > > (defvar tchanges-script > (expand-file-name > @@ -51,12 +57,13 @@ > > ;; Global value used during import, the local in the comment editor. > (defvar tchanges-buffer nil) > + > ;; Separate uses for the org buffer and the comment editor. > (defvar-local tchanges-comments nil) > > (defun tchanges--sentinel (process event) > - "Sentinel for pandoc process completion." > - (if (not (string-match "finished" event)) > + "Sentinel for pandoc process completion." ;checkdoc would like some m= ore documentation! > + (if (not (string-match "finished" event)) ;(elisp) Sentinels says it = should be "finished\n". Or use `process-status'. > (error "%s %s" process event) > (message "tchanges: Running pandoc...done") > (tchanges--postproc))) > @@ -71,17 +78,18 @@ > (setq tchanges-buffer nil)) > > (defun tchanges--postproc-2 (tag) > + ;; Documentation missing here! Flymake + checkdoc should warn you ab= out this. > (let ((insdel-regexp ".\\([id]\\).[^.]*?.[^.]*?.\\([^.]*?\\)=1F") > (comments-regexp ".[se][^.]*?.") > (bookmark-make-record-function #'tchanges-bookmark-make-record) > (bookmark-use-annotations nil) > bookmark-alist > (count 0)) > - (cl-letf (((symbol-function #'bookmark--set-fringe-mark) > - (symbol-function #'ignore))) > + (cl-letf (((symbol-function #'bookmark--set-fringe-mark) #'ignore)) > (goto-char (point-min)) > (while > (re-search-forward > + ;; I think that using `rx' would make this more readable and= maintainable. > ".s\\([^.]*?\\).\\([^.]*?\\).\\([^.]*?\\).\\([^.=1D]*?\\)." Me too; but it could wait, right? I was being conservative. > nil t) > (let* ((beg (match-beginning 0)) > @@ -142,13 +150,14 @@ > > ;;;###autoload > (defun tchanges-find-file (file &optional format) > + ;; checkdoc warns you that the first sentence should fit on a line! > "Convert a word processor document FILE with \\=3D'tracked changes\\= =3D' > to pre- and post- buffers. These may be `diff'ed or, more conveniently, > `ediff3'd with the original version of the file before it was shared (a= s > \\=3D'ancestor\\=3D'), inorder to hide (or nullify) common changes (the= ones > related to conversion). > > -(For even more accuracy, you may \\=3D'regenerate\\=3D' the above origi= nal > +\(For even more accuracy, you may \\=3D'regenerate\\=3D' the above orig= inal > version by a reverse conversion using the same Pandoc arguments > immediately after the forward conversion) > > @@ -180,10 +189,10 @@ Comment regions are highlighted with tooltips, but= also see > (defun tchanges-inline-comments () > "Prepare the buffer by inlining the comments, for export or saving." > (interactive) > - (if (or (not (bound-and-true-p tchanges-comments))) > + (if (or (not (bound-and-true-p tchanges-comments))) ;isn't it > always bound? > (user-error "No comments in buffer.") > (let ((bookmark-alist > - (sort tchanges-comments > + (sort tchanges-comments ;the :lessp requires Emacs 30, = but you could also call `sort' without keyword parameters > :lessp > (lambda (x y) > (> (bookmark-get-position x) > @@ -239,7 +248,7 @@ Comment regions are highlighted with tooltips, but a= lso see > Copied from the default function." > (let* ((posn (or (and (use-region-p) (region-beginning)) > (point))) > - (len (when (use-region-p) (- (region-end) posn)))) > + (len (and (use-region-p) (- (region-end) posn)))) ;is it ok if= this is nil? Yes, but it also needed some other changes to handle zero-width comments, which I've now made. > `((buffer . ,(current-buffer)) > (front-context-string > . ,(if (>=3D (- (point-max) (point)) > @@ -263,7 +272,7 @@ Copied from the default function." > (handler . ,#'tchanges--bookmark-handler)))) > > (defun tchanges-edit-annotation-confirm () > - "TODO" > + "TODO" ;TODO > (interactive) > (let* ((buf tchanges-buffer) > (newp (string-prefix-p "g" bookmark-annotation-name)) > @@ -318,16 +327,15 @@ Copied from the default function." > (bookmark-use-annotations t) > bookmark-alist > (buf (current-buffer))) > - (cl-letf (((symbol-function #'bookmark--set-fringe-mark) > - (symbol-function #'ignore))) > + (cl-letf (((symbol-function #'bookmark--set-fringe-mark) #'ignore)) > (bookmark-set (symbol-name (gensym)) t)) > ;; Single bookmark > (setq-local tchanges-buffer buf) > (setq tchanges-comments bookmark-alist) > (tchanges--remap '(bookmark-edit-annotation-confirm)))) > > -(defun tchanges-bmenu--revert () > - "Re-populate `tabulated-list-entries'. (Copied from the bookmark-...)= " > +(defun tchanges-bmenu--revert () ;Copied from the bookmark-... > + "Re-populate `tabulated-list-entries'." > (setq tabulated-list-entries nil) > (dolist (full-record bookmark-alist) > (let* ((name (bookmark-name-from-full-record full-record)) > @@ -354,8 +362,9 @@ Copied from the default function." > (tabulated-list-print t)) > > (defun tchanges-list-comments () > - "View buffer with all the comments. May be run in the buffer, after u= sing > -`tchanges-comment'. If point is at a comment, jump to it in the listing= ." > + "View buffer with all the comments. > +May be run in the buffer, after using `tchanges-comment'. If point is = at > +a comment, jump to it in the listing." > (interactive) > (let ((id (get-char-property (point) 'tchanges-comment)) > (buf (current-buffer)) > @@ -402,9 +411,9 @@ Copied from the default function." > tchanges-buffer)) > bookmark-watch-bookmark-file) > (cl-letf (((symbol-function #'bookmark--remove-fringe-mark) > - (symbol-function #'tchanges--remove-highlight)) > + #'tchanges--remove-highlight) > ((symbol-function #'bookmark-bmenu-list) > - (symbol-function #'tchanges-bmenu--revert))) > + #'tchanges-bmenu--revert)) > (bookmark-bmenu-execute-deletions)) > (with-current-buffer tchanges-buffer > (setq tchanges-comments bookmark-alist)))) > > > >> >> Here's a short screencast (.gif) demonstrating most of the features: >> >> https://codeberg.org/attachments/5096c13b-ff1f-47f6-8741-f3d955e34211 > > I am afraid this demonstration is not that useful if you don't already > understand what is going on. If you believe a visual introduction to be > useful, I would recommend preparing a recorded version with narration > and uploading it to a Peertube server. Is this better? I've improved it by referring to the README as a guide: https://codeberg.org/attachments/d9ce4f60-0acc-4316-ae45-dab6d59adc1e Regards, James