unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Philip Kaludercic <philipk@posteo.net>
To: Akib Azmain Turja <akib@disroot.org>
Cc: Emacs Developer List <emacs-devel@gnu.org>
Subject: Re: [NonGNU ELPA] 11 new packages!
Date: Fri, 25 Nov 2022 19:07:53 +0000	[thread overview]
Message-ID: <87v8n2zwc6.fsf@posteo.net> (raw)
In-Reply-To: <875yfdd5ad.fsf@disroot.org> (Akib Azmain Turja's message of "Thu, 17 Nov 2022 20:28:10 +0600")

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>
>>>> I'd like to add them, but I'll have to take the time to review them
>>>> first, which might take a bit.
>>>
>>> What do you want to review?  The patches, or the packages?
>>
>> The packages, unless you aren't interested in comments.
>
> Review if you wish, I welcome feedback.

Few more comments:

gnu-indent:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: comments.diff --]
[-- Type: text/x-diff, Size: 2890 bytes --]

diff --git a/gnu-indent.el b/gnu-indent.el
index 1a37851e96..8aea4161dc 100644
--- a/gnu-indent.el
+++ b/gnu-indent.el
@@ -51,18 +51,17 @@
 ;; Autoload so that users can set it as file local variable without
 ;; warning.
 ;;;###autoload
-(progn
-  (defcustom gnu-indent-options nil
-    "Arguments to pass to GNU Indent."
-    :type '(repeat string)
-    :safe (lambda (val)
-            (let ((valid t))
-              (while (and valid val)
-                (unless (stringp (car val))
-                  (setq valid nil))
-                (setq val (cdr val)))
-              valid))
-    :group 'gnu-indent))
+(defcustom gnu-indent-options nil
+  "Arguments to pass to GNU Indent."
+  :type '(repeat string)
+  :safe (lambda (val)
+          (let ((valid t))
+            (while (and valid val)
+              (unless (stringp (car val))
+                (setq valid nil))
+              (setq val (cdr val)))
+            valid))
+  :group 'gnu-indent)
 
 ;;;###autoload
 (defun gnu-indent-region (beg end)
@@ -88,14 +87,15 @@ When called non-interactively, indent text between BEG and END."
             (send-region process beg end)
             (process-send-eof process)
             (redisplay)
-            (while (process-live-p process)
-              (sleep-for 0.01))
+            (while (accept-process-output process nil 10))
             (unless (eq (process-exit-status process) 0)
-              (display-buffer (process-buffer process))
+              (pop-to-buffer (process-buffer process))
               (error "GNU Indent exited with non-zero status"))
             (save-restriction
               (let ((inhibit-read-only t))
                 (narrow-to-region beg end)
+		;; Perhaps something should be done to try an preserve
+		;; the point after indentation?
                 (insert-file-contents temp-file nil nil nil
                                       t))))
         (delete-file temp-file)))
@@ -108,11 +108,24 @@ When called non-interactively, indent text between BEG and END."
   (interactive)
   (gnu-indent-region (point-min) (point-max)))
 
+;; A little suggestion
+;;;###autoload
+(defun gnu-indent-defun-or-fill (arg)
+  "Indent current function with GNU Indent.
+If point is in a comment, call `fill-paragraph' instead.  A
+prefix argument ARG is passed to `fill-paragraph'."
+  (interactive "P")
+  (if (nth 8 (syntax-ppss))           ;if in a comment
+      (fill-paragraph arg)
+    (let ((bounds (bounds-of-thing-at-point 'defun)))
+      (if (consp bounds)
+	  (gnu-indent-region (car bounds) (cdr bounds))
+	(user-error "No defun at point")))))
+
 ;;;###autoload
 (define-minor-mode gnu-indent-mode
   "Indent buffer automatically with GNU Indent."
   :lighter " GNU-Indent"
-  :keymap nil
   (if gnu-indent-mode
       (add-hook 'before-save-hook #'gnu-indent-buffer nil t)
     (remove-hook 'before-save-hook #'gnu-indent-buffer t)))

[-- Attachment #3: Type: text/plain, Size: 11 bytes --]


devhelp:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: comments.diff --]
[-- Type: text/x-diff, Size: 2135 bytes --]

diff --git a/devhelp.el b/devhelp.el
index 6b3d9a1ce9..05aeb1e18e 100644
--- a/devhelp.el
+++ b/devhelp.el
@@ -48,6 +48,10 @@
 ;;             "~/.guix-profile/share/doc/"
 ;;             "~/.guix-profile/share/gtk-doc/html/"))
 
+;; Do you think it makes sense to automatically detect this (if the
+;; user has a ~/.guix-profile directory) and make the changes to the
+;; default value?
+
 ;; You can also bookmark pages, with the standard `bookmark-set' function.
 
 ;;; Code:
@@ -89,7 +93,7 @@ Integer means use that many columns.  Nil means use full window width."
 
 Note that on GNU Guix, Nix or other FHS (Filesystem Hierarchy Standard)
 non-compliant distributions, the default value won't work.  For GNU Guix,
-set it to '(\"/run/current-system/profile/share/doc/\"
+set it to \\='(\"/run/current-system/profile/share/doc/\"
 \"/run/current-system/profile/share/gtk-doc/html/\"
 \"~/.guix-profile/share/doc/\" \"~/.guix-profile/share/gtk-doc/html/\")."
   :type '(repeat directory))
@@ -147,6 +151,7 @@ absolute path to it."
                    ,(expand-file-name (dom-attr sec 'link) base)
                    ,(mapcar #'process-section (children-by-tag
                                                sec 'sub)))))
+      ;; `List' would be better here, right?
       `(,(or (dom-attr dom 'title) "Untitled")
         ,(or (dom-attr dom 'name) (file-name-base file))
         ,(or (dom-attr dom 'language) "any")
@@ -219,6 +224,7 @@ If a single file was opened, only show that book's table of contents."
 See `devhelp-toc' for more details."
   (let ((inhibit-read-only t))
     (erase-buffer)
+    ;; Why not prepare the document in SXML and then use `dom-print'?
     (insert
      "<html><head><title>Table of contents</title></head><body><ul>"
      (let ((book-tocs
@@ -447,6 +453,8 @@ When BASE is given, use it to make relative URLs absolute."
     (recenter)))
 
 (defun devhelp-history-back (&optional n)
+  ;; Is the variable actually optional, or won't the + raise a wrong
+  ;; type signal if invoked without an argument?
   "Go to the previous page.
 
 When prefix argument N is given, go to Nth previous page."

[-- Attachment #5: Type: text/plain, Size: 12 bytes --]


why-this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: comments.diff --]
[-- Type: text/x-diff, Size: 4249 bytes --]

diff --git a/why-this.el b/why-this.el
index 287ee99921..19acb90f9f 100644
--- a/why-this.el
+++ b/why-this.el
@@ -36,6 +36,10 @@
 (require 'timezone)
 (require 'color)
 
+;; A very high-level request that would probably take too much work to
+;; actualise, would be to figure out some way to integrate this into
+;; VC.  That way you can make use of this even if you don't use Git or Hg.
+
 (defgroup why-this nil
   "Show why the current line contains this."
   :group 'tools
@@ -64,6 +68,7 @@ the first argument is the command (which is a symbol):
     `:author'   Name of the author.
     `:time'     Time of change (local).
     `:desc'     Single line description of change."
+  ;; Would it make sense to use a `cl-defstruct'?
   :type 'hook
   :options (list #'why-this-git
                  #'why-this-hg)
@@ -326,6 +331,7 @@ TIME-FORMAT is used to format data."
              (?t . (why-this-format-time
                     time-format (plist-get data :time)))
              (?i . (plist-get data :desc)))))
+      ;; `format-spec' should make this simpler
       (replace-regexp-in-string
        "%."
        (lambda (str)
@@ -342,7 +348,7 @@ TIME-FORMAT is used to format data."
 (defun why-this--overlay-bg-type (pos)
   "Return the background type for overlay at POS."
   (cond
-   ((and (use-region-p)
+   ((and (use-region-p)                         ;perhaps `region-active-p'?
          (>= pos (region-beginning))
          (< pos (region-end)))
     'region)
@@ -364,6 +370,7 @@ TIME-FORMAT is used to format data."
                    'solaire-region-face
                  'region))
               ('line
+               ;; Looks like a `cond' to me
                (if (bound-and-true-p hl-line-mode)
                    (if (bound-and-true-p solaire-mode)
                        'solaire-hl-line-face
@@ -382,12 +389,10 @@ TIME-FORMAT is used to format data."
   (while why-this--overlays
     (delete-overlay (pop why-this--overlays)))
   (when why-this-mode
-    (let* ((begin (line-number-at-pos (if (use-region-p)
-                                          (region-beginning)
-                                        (point))))
-           (end (1+ (line-number-at-pos (if (use-region-p)
-                                            (region-end)
-                                          (point)))))
+    (let* ((line (line-number-at-pos (if (use-region-p)
+                                         (region-beginning)
+                                       (point))))
+           (begin line) (end (1+ line))
            (backend why-this--backend)
            (data (funcall backend 'line-data begin end)))
       (dolist (i (number-sequence 0 (- end begin 1)))
@@ -446,6 +451,8 @@ TIME-FORMAT is used to format data."
                                          (region-end)
                                        (point))))))
     (setq
+     ;; Perhaps this could be made more readble by using
+     ;; `thread-last' or the above let binding
      why-this--overlays
      (delq
       nil
@@ -458,10 +465,10 @@ TIME-FORMAT is used to format data."
                          (< line end)
                          (eq line (overlay-get ov 'why-this-line)))))
              (progn
-               (let* ((ov-start (overlay-start ov))
-                      line-begin
-                      line-end
-                      column)
+               (let ((ov-start (overlay-start ov))
+                     line-begin
+                     line-end
+                     column)
                  (save-excursion
                    (goto-char ov-start)
                    (setq line-begin (line-beginning-position))
@@ -537,6 +544,11 @@ Actually the supported backend is returned."
                    (* (- (nth i b-color)
                          (nth i a-color))
                       ratio)))))
+    ;; Note that RGB interpolation doesn't always behave the way you
+    ;; think it does.  You'd have to convert it into some other color
+    ;; space like CIELAB to get perceptual mixing right (but even that
+    ;; is trying because it requires some kind of a white-reference
+    ;; point).
     (color-rgb-to-hex (funcall mix 0)
                       (funcall mix 1)
                       (funcall mix 2)

[-- Attachment #7: Type: text/plain, Size: 288 bytes --]


A general comment might be that adding a .dir-locals.el file that
regulates Emcas' whitespace behaviour would be nice to have.

Also, I don't think you need to distribute the texinfo.tex files with
your packages, or at the least you could mark them as ignorable in the
.elpaignore file.

  parent reply	other threads:[~2022-11-25 19:07 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-14  7:42 [NonGNU ELPA] 11 new packages! Akib Azmain Turja
2022-11-15 17:42 ` Akib Azmain Turja
2022-11-15 19:53 ` Filipp Gunbin
2022-11-16 12:22   ` Akib Azmain Turja
2022-11-16 16:53     ` Eli Zaretskii
2022-11-16 17:43       ` Akib Azmain Turja
2022-11-16 19:47         ` Eli Zaretskii
2022-11-17 14:27           ` Akib Azmain Turja
2022-11-17 18:41             ` Stefan Monnier
2022-11-17 18:51             ` Yuan Fu
2022-11-24 23:38               ` Richard Stallman
2022-11-17 18:56             ` Eli Zaretskii
2022-11-15 19:57 ` Philip Kaludercic
2022-11-16 12:25   ` Akib Azmain Turja
2022-11-16 16:07     ` Philip Kaludercic
2022-11-16 17:45       ` Akib Azmain Turja
2022-11-16 18:19         ` Philip Kaludercic
2022-11-17 14:28           ` Akib Azmain Turja
2022-11-21 18:32             ` Philip Kaludercic
2022-11-22 15:20               ` Akib Azmain Turja
2022-11-22 17:07                 ` Philip Kaludercic
2022-11-22 17:42                   ` Akib Azmain Turja
2022-11-25  8:29                     ` Akib Azmain Turja
2022-11-25 16:32                       ` Philip Kaludercic
2022-11-25 17:14                         ` Akib Azmain Turja
2022-11-22 21:16                 ` Stefan Monnier
2022-11-25  7:14             ` Philip Kaludercic
2022-11-25  7:22               ` Philip Kaludercic
2022-11-25 12:45                 ` Akib Azmain Turja
2022-11-25 13:07               ` Akib Azmain Turja
2022-11-25 17:16                 ` Akib Azmain Turja
2022-11-25 17:31                 ` Philip Kaludercic
2022-11-26  5:53                   ` Akib Azmain Turja
2022-11-26 20:12                     ` Stefan Monnier
2022-11-25 19:07             ` Philip Kaludercic [this message]
2022-11-26  7:49               ` Akib Azmain Turja
2022-11-27  8:11                 ` Philip Kaludercic
2022-11-27 19:22                   ` Akib Azmain Turja
2022-11-27 19:55                   ` Akib Azmain Turja
2022-11-27 20:30                     ` Philip Kaludercic
2022-11-26 18:44               ` Akib Azmain Turja
2022-11-26 20:07             ` Philip Kaludercic
2022-11-26 20:17               ` Philip Kaludercic
2022-11-26 21:12                 ` Akib Azmain Turja
2022-11-27 11:45                   ` Philip Kaludercic
2022-11-27 17:04                     ` Akib Azmain Turja
2022-11-27 20:26                       ` Philip Kaludercic
2022-11-28 19:07                       ` Akib Azmain Turja
2022-11-28 19:42                         ` Philip Kaludercic
2022-11-28 20:32                           ` Akib Azmain Turja
2022-11-28 20:57                         ` Stefan Monnier
2022-11-27 18:40                   ` [NonGNU ELPA] 12 " Akib Azmain Turja
2022-11-27 20:36                     ` Philip Kaludercic
2022-11-27 21:11                       ` Akib Azmain Turja
2022-11-27  6:40               ` [NonGNU ELPA] 11 " Akib Azmain Turja
2022-11-19 12:05     ` Richard Stallman
2022-11-19 12:17       ` Philip Kaludercic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87v8n2zwc6.fsf@posteo.net \
    --to=philipk@posteo.net \
    --cc=akib@disroot.org \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).