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: Akib Azmain Turja
Subject: Re: [NonGNU ELPA] 11 new packages!
Date: Mon, 28 Nov 2022 01:55:29 +0600

Philip Kaludercic <philipk@posteo.net> writes:

> 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.
>

I don't like synchronous processes, they block Emacs completely.

>>>                (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/"))))
>

I like this approach.  Thanks.

>>> @@ -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))
>

At the first glance, I though your code is simply wrong.  But after
reading more attentively, it tells me you're actually a genius.

>>> @@ -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).

-- 
Akib Azmain Turja, GPG key: 70018CE5819F17A3BBA666AFE74F0EFA922AE7F5
Fediverse: akib@hostux.social
Codeberg: akib
emailselfdefense.fsf.org | "Nothing can be secure without encryption."

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]