Akib Azmain Turja writes: > Philip Kaludercic 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."