From: Akib Azmain Turja <akib@disroot.org>
To: Philip Kaludercic <philipk@posteo.net>
Cc: Emacs Developer List <emacs-devel@gnu.org>
Subject: Re: [NonGNU ELPA] 11 new packages!
Date: Tue, 29 Nov 2022 01:07:25 +0600 [thread overview]
Message-ID: <87sfi2vqxe.fsf@disroot.org> (raw)
In-Reply-To: <87cz98nxah.fsf@disroot.org> (Akib Azmain Turja's message of "Sun, 27 Nov 2022 23:04:54 +0600")
[-- Attachment #1: Type: text/plain, Size: 17761 bytes --]
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.
>> \f
>> ;;;;; 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."
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
next prev parent reply other threads:[~2022-11-28 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
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 [this message]
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=87sfi2vqxe.fsf@disroot.org \
--to=akib@disroot.org \
--cc=emacs-devel@gnu.org \
--cc=philipk@posteo.net \
/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).