[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: |
Tue, 29 Nov 2022 01:07:25 +0600 |
Akib Azmain Turja <akib@disroot.org> writes:
> Philip Kaludercic <philipk@posteo.net> writes:
[...]
>> @@ -445,6 +448,10 @@ If your process is choking on big inputs, try lowering
>> the value."
>>
>> (put 'eat-term-color-bright-white 'face-alias 'eat-term-color-15)
>>
>> +;; Perhaps you could automatically generate this block and make it
>> +;; more maintainable? `defface' is just a wrapper around
>> +;; `custom-declare-face', so you could invoke that in a loop that
>> +;; defines all the faces.
>> (defface eat-term-color-16
>> '((t :foreground "#000000" :background "#000000"))
>> "Face used to render text with 16th color of 256 color palette."
>
> Yes, you are right. I didn't know this at the time of writing that
> piece of code.
TODO.
[...]
>> @@ -1775,7 +1785,7 @@ Treat LINE FEED (?\\n) as the line delimiter."
>> ;; Move to the beginning of line, record the point, and return that
>> ;; point and the distance of that point from current line in lines.
>> (save-excursion
>> - (let* ((moved (eat--t-goto-eol n)))
>> + (let ((moved (eat--t-goto-eol n)))
>> (cons (point) moved))))
>>
>> (defun eat--t-col-motion (n)
>
> I think I do that very often. TODO. (A reminder for me. I'll do any
> change you suggested later, as I'm working to fix a bug in Eat.)
Done.
>
>> @@ -1823,9 +1833,9 @@ Assume all characters occupy a single column."
>> (eat--t-col-motion n))
>>
>> (defun eat--t-repeated-insert (c n &optional face)
>> - "Insert C, N times.
>> + "Insert character C, N times.
>>
>> -C is a character. FACE is the face to use, or nil."
>> +FACE is the face to use, or nil."
>> (let ((str (make-string n c)))
>> (insert (if face (propertize str 'face face) str))))
>>
>
Done.
[...]
>> (defun eat--t-cur-left (&optional n)
>> @@ -2297,6 +2308,7 @@ of range, place cursor at the edge of display."
>> "Change character set to CHARSET.
>>
>> CHARSET should be one of `g0', `g1', `g2' and `g3'."
>> + (cl-assert (memq charset '(g0 g1 g2 g3)))
>> (setf (car (eat--t-term-charset eat--t-term)) charset))
>>
>
> Thanks, that assertion should be there. TODO.
>
Done.
>> (defun eat--t-write (str)
>> @@ -2319,6 +2331,7 @@ CHARSET should be one of `g0', `g1', `g2' and `g3'."
>> ('dec-line-drawing
>> (let ((s (copy-sequence str)))
>> (dotimes (i (length s))
>> + ;; Perhaps you should pull out the alist into a `defconst'
>> (let ((replacement (alist-get (aref s i)
>> '((?+ . ?→)
>> (?, . ?←)
>
> Yes, I'm planning to use a hash-table (made with eval-when-compile).
> TODO.
>
Done, with a lot of changes in that function.
[...]
>> @@ -2561,7 +2574,7 @@ N defaults to 0. When N is 0, erase cursor to end of
>> line. When N is
>> (let* ((disp (eat--t-term-display eat--t-term))
>> (face (eat--t-term-face eat--t-term))
>> (cursor (eat--t-disp-cursor disp)))
>> - (pcase n
>> + (pcase-exhaustive n ;
>> ((or 0 'nil (pred (< 2)))
>> ;; Delete cursor position (inclusive) to end of line.
>> (delete-region (point) (car (eat--t-eol)))
>> @@ -2619,7 +2632,7 @@ to (1, 1). When N is 3, also erase the scrollback."
>> (let* ((disp (eat--t-term-display eat--t-term))
>> (face (eat--t-term-face eat--t-term))
>> (cursor (eat--t-disp-cursor disp)))
>> - (pcase n
>> + (pcase-exhaustive n
>> ((or 0 'nil (pred (< 3)))
>> ;; Delete from cursor position (inclusive) to end of terminal.
>> (delete-region (point) (point-max))
>
> What's... Checking docs... Oh, thanks, that'll trigger an error in
> case of no match. TODO.
>
Done.
[...]
>> @@ -2906,18 +2919,16 @@ position."
>> ;; on failure.
>> (when (and (<= scroll-begin (eat--t-cur-y cursor) scroll-end)
>> (not (zerop n)))
>> + (eat--t-goto-bol)
>> + (eat--t-repeated-insert ?\ (1- (eat--t-cur-x cursor))
>> + (and (eat--t-face-bg face)
>> + (eat--t-face-face face)))
>> (goto-char
>> - (prog1
>> - (progn
>> - ;; This function doesn't move the cursor, but pushes all
>> - ;; the line below and including current line. So to keep
>> - ;; the cursor unmoved, go to the beginning of line and
>> - ;; insert enough spaces to not move the cursor.
>> - (eat--t-goto-bol)
>> - (eat--t-repeated-insert ?\ (1- (eat--t-cur-x cursor))
>> - (and (eat--t-face-bg face)
>> - (eat--t-face-face face)))
>> - (point))
>> + ;; This function doesn't move the cursor, but pushes all
>> + ;; the line below and including current line. So to keep
>> + ;; the cursor unmoved, go to the beginning of line and
>> + ;; insert enough spaces to not move the cursor.
>> + (prog1 (point)
>> ;; Insert N lines.
>> (if (not (eat--t-face-bg face))
>> (eat--t-repeated-insert ?\n n)
>
> I really wonder why, why I wrote the code like that?
>
Fixed.
>> @@ -3189,7 +3200,8 @@ TOP defaults to 1 and BOTTOM defaults to the height of
>> the display."
>> nil t)))))
>> ;; Update face according to the attributes.
>> (setf (eat--t-face-face face)
>> - `(,@(when-let ((fg (or (if (eat--t-face-conceal face)
>> + ;; `while' is for side-effects, `and' is for expressions
>> + `(,@(and-let* ((fg (or (if (eat--t-face-conceal face)
>> (eat--t-face-bg face)
>> (eat--t-face-fg face))
>> (cond
>> @@ -3201,31 +3213,31 @@ TOP defaults to 1 and BOTTOM defaults to the height
>> of the display."
>> :background
>> :foreground)
>> fg))
>> - ,@(when-let ((bg (or (eat--t-face-bg face)
>> + ,@(and-let* ((bg (or (eat--t-face-bg face)
>> (and (eat--t-face-inverse face)
>> (face-background 'default)))))
>> (list (if (eat--t-face-inverse face)
>> :foreground
>> :background)
>> bg))
>> - ,@(when-let ((underline (eat--t-face-underline face)))
>> + ,@(and-let* ((underline (eat--t-face-underline face)))
>> (list
>> :underline
>> (list :color (eat--t-face-underline-color face)
>> :style underline)))
>> - ,@(when-let ((crossed (eat--t-face-crossed face)))
>> + ,@(and-let* ((crossed (eat--t-face-crossed face)))
>> ;; REVIEW: How about colors? No terminal supports
>> ;; crossed attribute with colors, so we'll need to be
>> ;; creative to add the feature.
>> `(:strike-through t))
>> :inherit
>> - (,@(when-let ((intensity (eat--t-face-intensity face)))
>> + (,@(and-let* ((intensity (eat--t-face-intensity face)))
>> (list intensity))
>> - ,@(when-let ((italic (eat--t-face-italic face)))
>> + ,@(and-let* ((italic (eat--t-face-italic face)))
>> (cl-assert (1value (eq (1value italic)
>> 'eat-term-italic)))
>> (list (1value italic)))
>> - ,@(when-let ((blink (eat--t-face-blink face)))
>> + ,@(and-let* ((blink (eat--t-face-blink face)))
>> (list blink))
>> ,(eat--t-face-font face))))))
>>
>
> Did you mean "when" instead of "while"? TODO.
>
Done.
>> @@ -3261,7 +3273,7 @@ MODE should be one of nil and `x10', `normal',
>> `button-event',
>> (setf (eat--t-term-mouse-pressed eat--t-term) nil))
>> ;; Inform the UI.
>> (funcall (eat--t-term-grab-mouse-fn eat--t-term) eat--t-term
>> - (pcase mode
>> + (pcase-exhaustive mode
>> ('x10 :click)
>> ('normal :modifier-click)
>> ('button-event :drag)
>
> TODO.
>
Done.
[...]
>> @@ -3788,17 +3802,15 @@ DATA is the selection data encoded in base64."
>> (rx ?\\))
>> output index)))
>> (if (not match)
>> - (progn
>> - ;; Not found, store the text to process it later when
>> - ;; we get the end of string.
>> - (setf (eat--t-term-parser-state eat--t-term)
>> - `(,state ,(concat buf (substring output
>> - index))))
>> - (setq index (length output)))
>> + ;; Not found, store the text to process it later when
>> + ;; we get the end of string.
>> + (setf (eat--t-term-parser-state eat--t-term)
>> + `(,state ,(concat buf (substring output
>> + index)))
>> + index (length output))
>> ;; Matched! Get the string from the output and previous
>> ;; runs.
>> - (let ((str (concat buf (substring output index
>> - match))))
>> + (let ((str (concat buf (substring output index match))))
>> (setq index (match-end 0))
>> ;; Is it really the end of string?
>> (if (and (= (aref output match) ?\\)
>
> Somehow I prefer to use one setq for each variable. Is setting multiple
> at once faster? Benchmarking with "benchmark"... Yes, about 1.5 times.
> TODO.
>
Done, but I didn't combine setf and setq, only two setq's or two setf's.
[...]
>> @@ -3985,6 +3997,10 @@ DATA is the selection data encoded in base64."
>> "Setup the environment for TERMINAL and eval BODY in it."
>> (declare (indent 1))
>> `(let ((eat--t-term ,terminal))
>> + ;; This won't change much here, because the next line would
>> + ;; trigger a similar exception, but there might be some other
>> + ;; place where an explicit check could be of use.
>> + (cl-check-type eat--t-term eat--t-term)
>> (with-current-buffer (eat--t-term-buffer eat--t-term)
>> (save-excursion
>> (save-restriction
>
> Hmm, it's a good idea on check early.
>
I didn't find any such spot...
[...]
>> @@ -4399,7 +4416,7 @@ client process may get confused."
>> (let ((ch (pcase (event-convert-list
>> (append (remq 'meta mods)
>> (list base)))
>> - (?\C-\ ?\C-@)
>> + (?\C-\s ?\C-@)
>> (?\C-/ ?\C-?)
>> (?\C-- ?\C-_)
>> (c c))))
>
> OK, now I must admit I missed this. TODO.
>
Done.
>> @@ -4608,9 +4625,9 @@ keywords:
>> EXCEPTIONS is a list of key sequences to not bind. Don't use
>> \"M-...\" key sequences in EXCEPTIONS, use \"ESC ...\" instead."
>> (let ((map (make-sparse-keymap)))
>> - (cl-labels ((bind (key)
>> - (unless (member key exceptions)
>> - (define-key map key input-command))))
>> + (cl-flet ((bind (key)
>> + (unless (member key exceptions)
>> + (define-key map key input-command))))
>> (when (memq :ascii categories)
>> ;; Bind ASCII and self-insertable characters except ESC and
>> ;; DEL.
>
> Thanks.
>
Done.
>> @@ -4618,7 +4635,7 @@ EXCEPTIONS is a list of key sequences to not bind.
>> Don't use
>> (cl-loop
>> for i from ?\C-@ to ?~
>> do (unless (= i meta-prefix-char)
>> - (bind `[,i])))
>> + (bind (vector i))))
>> ;; Bind `backspace', `delete', `deletechar', and all modified
>> ;; variants.
>> (dolist (key '( backspace C-backspace
>
> Again a useless backquote usage...
>
Done.
>> @@ -4780,11 +4797,12 @@ return \"eat-color\", otherwise return \"eat-mono\"."
>> (defvar eat--fast-blink-timer nil
>> "Timer for blinking rapidly blinking text.")
>>
>> +(declare-function face-remap-add-relative "face-remap"
>> + (face &rest specs))
>> +(declare-function face-remap-remove-relative "face-remap" (cookie))
>> +
>> (defun eat--flip-slow-blink-state ()
>> "Flip the state of slowly blinking text."
>> - (declare-function face-remap-add-relative "face-remap"
>> - (face &rest specs))
>> - (declare-function face-remap-remove-relative "face-remap" (cookie))
>> (face-remap-remove-relative eat--slow-blink-remap)
>> (setq eat--slow-blink-remap
>> (face-remap-add-relative
>
> TODO.
>
>> @@ -4794,9 +4812,6 @@ return \"eat-color\", otherwise return \"eat-mono\"."
>>
>> (defun eat--flip-fast-blink-state ()
>> "Flip the state of rapidly blinking text."
>> - (declare-function face-remap-add-relative "face-remap"
>> - (face &rest specs))
>> - (declare-function face-remap-remove-relative "face-remap" (cookie))
>> (face-remap-remove-relative eat--fast-blink-remap)
>> (setq eat--fast-blink-remap
>> (face-remap-add-relative
Both, done.
>> @@ -4853,6 +4868,7 @@ return \"eat-color\", otherwise return \"eat-mono\"."
>> (face-remap-remove-relative eat--fast-blink-remap)
>> (remove-hook 'pre-command-hook #'eat--blink-stop-timers t)
>> (remove-hook 'post-command-hook #'eat--blink-start-timers t)
>> + ;; I think `mapc' comes in nicely here
>> (kill-local-variable 'eat--slow-blink-state)
>> (kill-local-variable 'eat--fast-blink-state)
>> (kill-local-variable 'eat--slow-blink-remap)
>
> Yes. TODO.
>
Done.
[...]
>> @@ -5216,13 +5233,13 @@ ARG is passed to `yank-pop', which see."
>> (defun eat-semi-char-mode ()
>> "Switch to semi-char mode."
>> (interactive)
>> - (if (not eat--terminal)
>> - (error "Process not running")
>> - (setq buffer-read-only nil)
>> - (eat--char-mode -1)
>> - (eat--semi-char-mode +1)
>> - (eat--grab-mouse nil eat--mouse-grabbing-type)
>> - (force-mode-line-update)))
>> + (unless eat--terminal
>> + (error "Process not running"))
>> + (setq buffer-read-only nil)
>> + (eat--char-mode -1)
>> + (eat--semi-char-mode +1)
>> + (eat--grab-mouse nil eat--mouse-grabbing-type)
>> + (force-mode-line-update))
>>
>
> Thanks. TODO.
>
Done.
>> (defun eat-char-mode ()
>> "Switch to char mode."
>> @@ -5302,7 +5319,7 @@ selection, or nil if none."
>>
>> (defun eat--bell (_)
>> "Ring the bell."
>> - (beep t))
>> + (ding t))
>>
>
> Any difference? Anyway, TODO.
>
Done.
>>
>> ;;;;; Major Mode.
>> @@ -5340,6 +5357,7 @@ END if it's safe to do so."
>> (define-derived-mode eat-mode fundamental-mode "Eat"
>> "Major mode for Eat."
>> :group 'eat-ui
>> + ;; You can abbreviate parts of the definition with `setq-local'.
>> (make-local-variable 'buffer-read-only)
>> (make-local-variable 'buffer-undo-list)
>> (make-local-variable 'filter-buffer-substring-function)
>> @@ -5372,8 +5390,8 @@ END if it's safe to do so."
>> (:propertize
>> "semi-char"
>> help-echo
>> - ,(concat "mouse-1: Switch to char mode, "
>> - "mouse-3: Switch to emacs mode")
>> + ,"mouse-1: Switch to char mode, \
>> +mouse-3: Switch to emacs mode"
>> mouse-face mode-line-highlight
>> local-map
>> (keymap
>
> TODO.
>
Done.
[...]
>> @@ -5974,6 +5996,8 @@ modify its argument to change the filter, the sentinel
>> and invoke
>> (expand-file-name command))
>> args))))
>> (apply make-process plist)
>> + ;; `plist-put' is not destructive, you need to store the
>> + ;; return value.
>> (plist-put plist :filter #'eat--eshell-filter)
>> (plist-put plist :sentinel #'eat--eshell-sentinel)
>> (plist-put
>
> Thanks, I'll use setf instead. TODO.
>
Done.
>> @@ -6328,7 +6352,7 @@ FN, `eat-exec', which see."
>> (push (cons var (symbol-value var)) variables)))
>> (with-current-buffer buf
>> (lisp-data-mode)
>> - (insert ";; -*- lisp-data -*-\n")
>> + (insert ";; -*- mode: lisp-data -*-\n") ;just a suggestion
>> (eat--trace-log time 'create 'eat width height
>> variables))))))
>>
>
> TODO.
>
Done.
[...]
--
Akib Azmain Turja, GPG key: 70018CE5819F17A3BBA666AFE74F0EFA922AE7F5
Fediverse: akib@hostux.social
Codeberg: akib
emailselfdefense.fsf.org | "Nothing can be secure without encryption."
signature.asc
Description: PGP signature
- Re: [NonGNU ELPA] 11 new packages!, (continued)
- Re: [NonGNU ELPA] 11 new packages!, Akib Azmain Turja, 2022/11/26
- Re: [NonGNU ELPA] 11 new packages!, Philip Kaludercic, 2022/11/26
- Re: [NonGNU ELPA] 11 new packages!, Philip Kaludercic, 2022/11/26
- Re: [NonGNU ELPA] 11 new packages!, Akib Azmain Turja, 2022/11/26
- Re: [NonGNU ELPA] 11 new packages!, Philip Kaludercic, 2022/11/27
- Re: [NonGNU ELPA] 11 new packages!, Akib Azmain Turja, 2022/11/27
- Re: [NonGNU ELPA] 11 new packages!, Philip Kaludercic, 2022/11/27
- Re: [NonGNU ELPA] 11 new packages!,
Akib Azmain Turja <=
- Re: [NonGNU ELPA] 11 new packages!, Philip Kaludercic, 2022/11/28
- Re: [NonGNU ELPA] 11 new packages!, Akib Azmain Turja, 2022/11/28
- Re: [NonGNU ELPA] 11 new packages!, Stefan Monnier, 2022/11/28
- Re: [NonGNU ELPA] 12 new packages!, Akib Azmain Turja, 2022/11/27
- Re: [NonGNU ELPA] 12 new packages!, Philip Kaludercic, 2022/11/27
- Re: [NonGNU ELPA] 12 new packages!, Akib Azmain Turja, 2022/11/27
- Re: [NonGNU ELPA] 11 new packages!, Akib Azmain Turja, 2022/11/27
- Re: [NonGNU ELPA] 11 new packages!, Richard Stallman, 2022/11/19
- Re: [NonGNU ELPA] 11 new packages!, Philip Kaludercic, 2022/11/19
Re: [NonGNU ELPA] 11 new packages!, Akib Azmain Turja, 2022/11/16