emacs-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [NonGNU ELPA] 11 new packages!

From: Philip Kaludercic
Subject: Re: [NonGNU ELPA] 11 new packages!
Date: Sun, 27 Nov 2022 08:11:27 +0000 
Akib Azmain Turja <akib@disroot.org> writes:

>>  ;;;###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))
>
> MILLISEC argument is obsolete, I used SECONDS instead.

Actually, this change might have been pointless.  I misremembed the
warning from (elisp) Accepting Output, that said you shouldn't combine
`accept-process-output' and `process-live-p', but what you were doing
was probably harmless.

I guess the real question is why you don't use a synchronous process?
indent usually isn't that slow.

>>                (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))))
>
> On my computer, the point doesn't move relative to text, because the
> fifth argument to insert-file-contents is t.

My bad then.

>>          (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")))))
>> +
>
> Great idea.  But would it cause problem to assign copyright if I take
> your change?  (AFAIK you've completed paperwork, but in this case you're
> not contributing to FSF-copyrighted code, so is this change covered by
> your paperwork?  Right now, no, you're the copyright holder.  (According
> to Section 2 of copyright assignment agreement sent to me.))

I don't think there should be an issue, as we want to add the package to
NonGNU ELPA.  If the package is later moved to GNU ELPA, then my CA
should cover it.

>> devhelp:
>>
>> 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?
>> +
>
> Yes, it makes sense.  But I didn't find any way to detected the
> directories, except heuristics.

What would the problem be with checking the existence of these specific
(or other well known directories)?

   (append [default value]
           (and (file-exists-p "~/.guix-profile/share/doc/"
                (list "~/.guix-profile/share/doc/")))
           (and (file-exists-p "~/.guix-profile/share/gtk-doc/html/")
                (list "~/.guix-profile/share/gtk-doc/html/"))))

>> @@ -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
>
> Hmm, that would be a cleaner approach.  Now I need to research the
> format of SXML.

This gives a brief demonstration:

(dom-print '(tag ((key . "value")) (sub-tag () "body")))
;; Output: <tag key="value"><sub-tag>body</sub-tag></tag>

>>  (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)
>
> Maybe.  But I think the plist approach is simpler and good enough.

My fear is that it is less robust, but my fear is probably unreasonable.

>> @@ -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
>
> Yes, but I think its more efficient.  (IIUC cond would test hl-line-mode
> before returning why-this-face.)

I don't think there would be a big difference.  The transformation I had
in mind was something along these lines:

(if foo (if bar (if baz 0 1) 2) 3)
⇝
(cond ((not foo) 3)
      ((not bar) 2)
      ((not baz) 1)
      (t 0))

>> @@ -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)
>
> Any edge case of my code?

Not an edge-case, just a mathematical/physical problem.  Maybe
https://www.alanzucconi.com/2016/01/06/colour-interpolation/ can help
explain what I had in mind (it also indicates that interpolation in the
HSV color space might be good enough of a fix).
reply via email to
[Prev in Thread] Current Thread [Next in Thread]