unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [NonGNU ELPA] 11 new packages!
@ 2022-11-14  7:42 Akib Azmain Turja
  2022-11-15 17:42 ` Akib Azmain Turja
                   ` (2 more replies)
  0 siblings, 3 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-14  7:42 UTC (permalink / raw)
  To: Emacs Developer List


[-- Attachment #1.1.1: Type: text/plain, Size: 744 bytes --]


TL;DR: New packages: Workroom, Blow, IWindow, Why This, Devhelp, GNU
Indent, HL Column, Minibar, testcover-mark-line, Camera and GC Buffers.

Before someone asks me to consider assigning my copyright, I would like
to say that a printed copy of assignment agreement is in my hands right
now, but I can't sign it now, and I don't know when I'll able to sign
(don't get me wrong, I really want to sign it).

* Workroom - Named rooms for work without irrelevant distracting buffers

Workroom is yet another workspace package.  It's similar to perspective
and persp-mode packages, but here is are the key advantanges:

+ Each workroom (workspace) can have multiple views (window
  configuration).
+ Bookmark integration.
+ Desktop.el integration.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: Workroom --]
[-- Type: text/x-patch, Size: 793 bytes --]

From e12d0fd3896d4fa0c885d78be929e6992e0f88b8 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 01:48:36 +0600
Subject: [PATCH] * elpa-packages (workroom): New package

---
 elpa-packages | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..62cf79a 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -663,6 +663,9 @@
 
  ("with-simulated-input" :url "https://github.com/DarwinAwardWinner/with-simulated-input")
 
+ ("workroom"            :url "https://codeberg.org/akib/emacs-workroom"
+  :doc "workroom.texi")
+
  ("ws-butler"           :url "https://github.com/lewang/ws-butler"
   :readme "README.md"
   :ignored-files ("COPYING" "tests" "Makefile" ".travis.yml"))
-- 
2.37.1


[-- Attachment #1.1.3: Type: text/plain, Size: 311 bytes --]


* Blow - Blow away mode lighters

This is similar to the Diminish package.  I didn't use Diminish, so I
can't compare.  However, the key difference of Blow from other similar
packages are that mode line is affected only when blow-mode is enabled
and the lighter is actually stored in a customizable variable.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.4: Blow --]
[-- Type: text/x-patch, Size: 700 bytes --]

From 34a638fe45b9b52470e88203d37f459b83cb8122 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 09:45:14 +0600
Subject: [PATCH] * elpa-packages (blow): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..fdea5e4 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -53,6 +53,8 @@
 
  ("bison-mode"		:url "https://github.com/Wilfred/bison-mode")
 
+ ("blow"                :url "https://codeberg.org/akib/emacs-blow")
+
  ("boxquote"            :url "https://github.com/davep/boxquote.el.git"
   :readme "README.md"
   :ignored-files ("COPYING"))
-- 
2.37.1


[-- Attachment #1.1.5: Type: text/plain, Size: 286 bytes --]


* IWindow - Interactively manipulate windows

This is similar to ace-window.  Only the key difference is that it uses
the mode line by default instead of Avy, which I need for EXWM.  However
it is extensible, almost anything can be used for showing the keys with
a few lines of code.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.6: IWindow --]
[-- Type: text/x-patch, Size: 688 bytes --]

From 6b85d539b486c70e1328d8ee6d876280ab5256f8 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 10:55:25 +0600
Subject: [PATCH] * elpa-packages (iwindow): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..2cf67d9 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -312,6 +312,8 @@
 
  ("inkpot-theme"        :url "https://codeberg.org/ideasman42/emacs-theme-inkpot")
 
+ ("iwindow"             :url "https://codeberg.org/akib/emacs-iwindow")
+
  ("j-mode"		:url "https://github.com/zellio/j-mode"
   :ignored-files ("LICENSE"))
 
-- 
2.37.1


[-- Attachment #1.1.7: Type: text/plain, Size: 246 bytes --]


* Why This - Why is this line here?  Ask version control

This shows the last commit that change the current line.  Works for Git
and Mercurial.  However integration with other packages like VC and
Magit is still WIP (due to lack of interest).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.8: Why This --]
[-- Type: text/x-patch, Size: 773 bytes --]

From 83e97fa5594a967fecd0f0bd108181ab0e8242ee Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 09:31:23 +0600
Subject: [PATCH] * elpa-packages (why-this): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..d2190f9 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -656,6 +656,8 @@
  ("wgrep"		:url "https://github.com/mhayashi1120/Emacs-wgrep"
   :ignored-files "COPYING")
 
+ ("why-this"            :url "https://codeberg.org/akib/emacs-why-this")
+
  ("with-editor"		:url "https://github.com/magit/with-editor"
   :ignored-files ("LICENSE" "htmlxref.cnf" ".travis.yml" ".mailmap" "Makefile")
   :lisp-dir "lisp"
-- 
2.37.1


[-- Attachment #1.1.9: Type: text/plain, Size: 156 bytes --]


* Devhelp - Browse documentation in Devhelp format

This can be used to read the Devhelp documentation that comes with
softwares like GTK, Pango, Python.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.10: Devhelp --]
[-- Type: text/x-patch, Size: 699 bytes --]

From b408ce846d4da44a78aa39f4faaad914dbef3989 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 09:54:29 +0600
Subject: [PATCH] * elpa-packages (devhelp): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..2dd9fad 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -98,6 +98,8 @@
   :ignored-files ("LICENSE" "test" "Cask" "Makefile")
   :news "CHANGELOG.md")
 
+ ("devhelp"             :url "https://codeberg.org/akib/emacs-devhelp")
+
  ("diff-ansi"   :url "https://codeberg.org/ideasman42/emacs-diff-ansi"
   :ignored-files ("LICENSE"))
 
-- 
2.37.1


[-- Attachment #1.1.11: Type: text/plain, Size: 134 bytes --]


* GNU Indent - Indent your code with GNU Indent

This indents your code with GNU Indent, either automatically on save or
on demand.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.12: GNU Indent --]
[-- Type: text/x-patch, Size: 765 bytes --]

From 97f1be13211835b35423875c51c973ef3df8e76e Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 01:52:17 +0600
Subject: [PATCH] * elpa-packages (gnu-indent): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..5f75dfe 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -218,6 +218,8 @@
   ;; :doc "texi/gnu-apl-mode.texi" ; the manual is currently empty
   :ignored-files ("Makefile"))
 
+ ("gnu-indent"	        :url "https://codeberg.org/akib/emacs-gnu-indent")
+
  ("gnuplot"		:url "https://github.com/emacs-gnuplot/gnuplot"
   :ignored-files ("LICENSE" "Makefile" "gpelcard.tex")
   :news "CHANGELOG.org")
-- 
2.37.1


[-- Attachment #1.1.13: Type: text/plain, Size: 206 bytes --]


* HL Column - Highlight the current column

As the summary line says, it highlight the current column.  I don't use
it personally.  Someone showed that Vim has this feature, so Emacs must
also have this.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.14: HL Column --]
[-- Type: text/x-patch, Size: 736 bytes --]

From c9bfc20a385410453544a9ad1915e940d5d2d977 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 10:02:20 +0600
Subject: [PATCH] * elpa-packages (hl-column): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..311614a 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -287,6 +287,8 @@
 
  ("hl-block-mode"       :url "https://codeberg.org/ideasman42/emacs-hl-block-mode")
 
+ ("hl-column"           :url "https://codeberg.org/akib/emacs-hl-column")
+
  ("htmlize"		:url "https://github.com/hniksic/emacs-htmlize"
   :ignored-files ("htmlize.el.html")
   :release-branch "stable")
-- 
2.37.1


[-- Attachment #1.1.15: Type: text/plain, Size: 193 bytes --]


* Minibar - Modular status bar in minibuffer

Since I use EXWM, I need a status bar for showing various information.
None of the external status bar programs satisfied me, so I wrote my
own.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.16: Minibar --]
[-- Type: text/x-patch, Size: 705 bytes --]

From f210f23f46ca3ba2439ccaa77d699417d2be7ed5 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 10:58:08 +0600
Subject: [PATCH] * elpa-packages (minibar): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..347e3b7 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -387,6 +387,8 @@
 
  ("mentor"              :url "https://github.com/skangas/mentor.git")
 
+ ("minibar"             :url "https://codeberg.org/akib/emacs-minibar")
+
  ("moe-theme"           :url "https://github.com/kuanyui/moe-theme.el.git"
   :ignored-files ("pics" "LICENSE"))
 
-- 
2.37.1


[-- Attachment #1.1.17: Type: text/plain, Size: 389 bytes --]


* testcover-mark-line - Mark whole line with Testcover

I developed this for my another package in development (you can't find
it on the internet), which has more than 6000 lines of code and more
than 50 tests trying to cover all non-UI parts of it.  Finding the
testcover marks in such a large file is extremely is hard for me, so I
wrote it to mark whole line also, with another face.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.18: testcover-mark-line --]
[-- Type: text/x-patch, Size: 745 bytes --]

From 58c95e5a1052d892f1ab5f5ddfc2bcf1bf77cce8 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 11:01:33 +0600
Subject: [PATCH] * elpa-packages (testcover-mark-line): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..588b885 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -609,6 +609,8 @@
   :readme "readme.org"
   :ignored-files ("COPYING" "screenshots"))
 
+ ("testcover-mark-line" :url "https://codeberg.org/akib/emacs-testcover-mark-line")
+
  ("textile-mode"	:url "https://github.com/juba/textile-mode")
 
  ("toc-org"             :url "https://github.com/snosov1/toc-org.git"
-- 
2.37.1


[-- Attachment #1.1.19: Type: text/plain, Size: 243 bytes --]


* Camera - Take picture with your camera

Turn your Emacs into a Camera!  I don't use the webcam much, so I didn't
want to install the heavy webcam programs and wrote this small package.
It can't record video, it can definitely take photos.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.20: Camera --]
[-- Type: text/x-patch, Size: 764 bytes --]

From 88445f5fe9cf914ce1d665ce1f432c051e59ac0e Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 09:52:35 +0600
Subject: [PATCH] * elpa-packages (camera): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..9548c68 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -60,6 +60,8 @@
  ("buttercup"		:url "https://github.com/jorgenschaefer/emacs-buttercup"
   :ignored-files ("LICENSE"))
 
+ ("camera"              :url "https://codeberg.org/akib/emacs-camera")
+
  ("caml"		:url "https://github.com/ocaml/caml-mode"
   :ignored-files ("COPYING")
   ;; The version 4.7.1 from Melpa-stable seems to correspond to
-- 
2.37.1


[-- Attachment #1.1.21: Type: text/plain, Size: 151 bytes --]


* GC Buffers - Kill garbage buffers automatically

This kills all garbage buffers (can be obviously customized) after a
certain amount of idle time.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.22: GC Buffers --]
[-- Type: text/x-patch, Size: 700 bytes --]

From 20f25aadddd45112e298c8f4ae07b31ab24161b3 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 14 Nov 2022 09:56:42 +0600
Subject: [PATCH] * elpa-packages (gc-buffers): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b0dd9d3..899fe24 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -185,6 +185,8 @@
 
  ("free-keys"           :url "https://github.com/Fuco1/free-keys")
 
+ ("gc-buffers"          :url "https://codeberg.org/akib/emacs-gc-buffers")
+
  ("geiser"		:url "https://gitlab.com/emacs-geiser/geiser.git"
   :lisp-dir "elisp"
   :readme "readme.org"
-- 
2.37.1


[-- Attachment #1.1.23: Type: text/plain, Size: 196 bytes --]


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

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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-15 19:57 ` Philip Kaludercic
  2 siblings, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-15 17:42 UTC (permalink / raw)
  To: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 12258 bytes --]


Looks like I missed the main thing I wanted to say.

I would like to publish 11 new packages to NonGNU package, as described
below:

Akib Azmain Turja <akib@disroot.org> writes:

> TL;DR: New packages: Workroom, Blow, IWindow, Why This, Devhelp, GNU
> Indent, HL Column, Minibar, testcover-mark-line, Camera and GC Buffers.
>
> Before someone asks me to consider assigning my copyright, I would like
> to say that a printed copy of assignment agreement is in my hands right
> now, but I can't sign it now, and I don't know when I'll able to sign
> (don't get me wrong, I really want to sign it).
>
> * Workroom - Named rooms for work without irrelevant distracting buffers
>
> Workroom is yet another workspace package.  It's similar to perspective
> and persp-mode packages, but here is are the key advantanges:
>
> + Each workroom (workspace) can have multiple views (window
>   configuration).
> + Bookmark integration.
> + Desktop.el integration.
>
> From e12d0fd3896d4fa0c885d78be929e6992e0f88b8 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 01:48:36 +0600
> Subject: [PATCH] * elpa-packages (workroom): New package
>
> ---
>  elpa-packages | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..62cf79a 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -663,6 +663,9 @@
>  
>   ("with-simulated-input" :url "https://github.com/DarwinAwardWinner/with-simulated-input")
>  
> + ("workroom"            :url "https://codeberg.org/akib/emacs-workroom"
> +  :doc "workroom.texi")
> +
>   ("ws-butler"           :url "https://github.com/lewang/ws-butler"
>    :readme "README.md"
>    :ignored-files ("COPYING" "tests" "Makefile" ".travis.yml"))
> -- 
> 2.37.1
>
>
> * Blow - Blow away mode lighters
>
> This is similar to the Diminish package.  I didn't use Diminish, so I
> can't compare.  However, the key difference of Blow from other similar
> packages are that mode line is affected only when blow-mode is enabled
> and the lighter is actually stored in a customizable variable.
>
> From 34a638fe45b9b52470e88203d37f459b83cb8122 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 09:45:14 +0600
> Subject: [PATCH] * elpa-packages (blow): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..fdea5e4 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -53,6 +53,8 @@
>  
>   ("bison-mode"		:url "https://github.com/Wilfred/bison-mode")
>  
> + ("blow"                :url "https://codeberg.org/akib/emacs-blow")
> +
>   ("boxquote"            :url "https://github.com/davep/boxquote.el.git"
>    :readme "README.md"
>    :ignored-files ("COPYING"))
> -- 
> 2.37.1
>
>
> * IWindow - Interactively manipulate windows
>
> This is similar to ace-window.  Only the key difference is that it uses
> the mode line by default instead of Avy, which I need for EXWM.  However
> it is extensible, almost anything can be used for showing the keys with
> a few lines of code.
>
> From 6b85d539b486c70e1328d8ee6d876280ab5256f8 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 10:55:25 +0600
> Subject: [PATCH] * elpa-packages (iwindow): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..2cf67d9 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -312,6 +312,8 @@
>  
>   ("inkpot-theme"        :url "https://codeberg.org/ideasman42/emacs-theme-inkpot")
>  
> + ("iwindow"             :url "https://codeberg.org/akib/emacs-iwindow")
> +
>   ("j-mode"		:url "https://github.com/zellio/j-mode"
>    :ignored-files ("LICENSE"))
>  
> -- 
> 2.37.1
>
>
> * Why This - Why is this line here?  Ask version control
>
> This shows the last commit that change the current line.  Works for Git
> and Mercurial.  However integration with other packages like VC and
> Magit is still WIP (due to lack of interest).
>
> From 83e97fa5594a967fecd0f0bd108181ab0e8242ee Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 09:31:23 +0600
> Subject: [PATCH] * elpa-packages (why-this): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..d2190f9 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -656,6 +656,8 @@
>   ("wgrep"		:url "https://github.com/mhayashi1120/Emacs-wgrep"
>    :ignored-files "COPYING")
>  
> + ("why-this"            :url "https://codeberg.org/akib/emacs-why-this")
> +
>   ("with-editor"		:url "https://github.com/magit/with-editor"
>    :ignored-files ("LICENSE" "htmlxref.cnf" ".travis.yml" ".mailmap" "Makefile")
>    :lisp-dir "lisp"
> -- 
> 2.37.1
>
>
> * Devhelp - Browse documentation in Devhelp format
>
> This can be used to read the Devhelp documentation that comes with
> softwares like GTK, Pango, Python.
>
> From b408ce846d4da44a78aa39f4faaad914dbef3989 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 09:54:29 +0600
> Subject: [PATCH] * elpa-packages (devhelp): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..2dd9fad 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -98,6 +98,8 @@
>    :ignored-files ("LICENSE" "test" "Cask" "Makefile")
>    :news "CHANGELOG.md")
>  
> + ("devhelp"             :url "https://codeberg.org/akib/emacs-devhelp")
> +
>   ("diff-ansi"   :url "https://codeberg.org/ideasman42/emacs-diff-ansi"
>    :ignored-files ("LICENSE"))
>  
> -- 
> 2.37.1
>
>
> * GNU Indent - Indent your code with GNU Indent
>
> This indents your code with GNU Indent, either automatically on save or
> on demand.
>
> From 97f1be13211835b35423875c51c973ef3df8e76e Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 01:52:17 +0600
> Subject: [PATCH] * elpa-packages (gnu-indent): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..5f75dfe 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -218,6 +218,8 @@
>    ;; :doc "texi/gnu-apl-mode.texi" ; the manual is currently empty
>    :ignored-files ("Makefile"))
>  
> + ("gnu-indent"	        :url "https://codeberg.org/akib/emacs-gnu-indent")
> +
>   ("gnuplot"		:url "https://github.com/emacs-gnuplot/gnuplot"
>    :ignored-files ("LICENSE" "Makefile" "gpelcard.tex")
>    :news "CHANGELOG.org")
> -- 
> 2.37.1
>
>
> * HL Column - Highlight the current column
>
> As the summary line says, it highlight the current column.  I don't use
> it personally.  Someone showed that Vim has this feature, so Emacs must
> also have this.
>
> From c9bfc20a385410453544a9ad1915e940d5d2d977 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 10:02:20 +0600
> Subject: [PATCH] * elpa-packages (hl-column): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..311614a 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -287,6 +287,8 @@
>  
>   ("hl-block-mode"       :url "https://codeberg.org/ideasman42/emacs-hl-block-mode")
>  
> + ("hl-column"           :url "https://codeberg.org/akib/emacs-hl-column")
> +
>   ("htmlize"		:url "https://github.com/hniksic/emacs-htmlize"
>    :ignored-files ("htmlize.el.html")
>    :release-branch "stable")
> -- 
> 2.37.1
>
>
> * Minibar - Modular status bar in minibuffer
>
> Since I use EXWM, I need a status bar for showing various information.
> None of the external status bar programs satisfied me, so I wrote my
> own.
>
> From f210f23f46ca3ba2439ccaa77d699417d2be7ed5 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 10:58:08 +0600
> Subject: [PATCH] * elpa-packages (minibar): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..347e3b7 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -387,6 +387,8 @@
>  
>   ("mentor"              :url "https://github.com/skangas/mentor.git")
>  
> + ("minibar"             :url "https://codeberg.org/akib/emacs-minibar")
> +
>   ("moe-theme"           :url "https://github.com/kuanyui/moe-theme.el.git"
>    :ignored-files ("pics" "LICENSE"))
>  
> -- 
> 2.37.1
>
>
> * testcover-mark-line - Mark whole line with Testcover
>
> I developed this for my another package in development (you can't find
> it on the internet), which has more than 6000 lines of code and more
> than 50 tests trying to cover all non-UI parts of it.  Finding the
> testcover marks in such a large file is extremely is hard for me, so I
> wrote it to mark whole line also, with another face.
>
> From 58c95e5a1052d892f1ab5f5ddfc2bcf1bf77cce8 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 11:01:33 +0600
> Subject: [PATCH] * elpa-packages (testcover-mark-line): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..588b885 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -609,6 +609,8 @@
>    :readme "readme.org"
>    :ignored-files ("COPYING" "screenshots"))
>  
> + ("testcover-mark-line" :url "https://codeberg.org/akib/emacs-testcover-mark-line")
> +
>   ("textile-mode"	:url "https://github.com/juba/textile-mode")
>  
>   ("toc-org"             :url "https://github.com/snosov1/toc-org.git"
> -- 
> 2.37.1
>
>
> * Camera - Take picture with your camera
>
> Turn your Emacs into a Camera!  I don't use the webcam much, so I didn't
> want to install the heavy webcam programs and wrote this small package.
> It can't record video, it can definitely take photos.
>
> From 88445f5fe9cf914ce1d665ce1f432c051e59ac0e Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 09:52:35 +0600
> Subject: [PATCH] * elpa-packages (camera): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..9548c68 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -60,6 +60,8 @@
>   ("buttercup"		:url "https://github.com/jorgenschaefer/emacs-buttercup"
>    :ignored-files ("LICENSE"))
>  
> + ("camera"              :url "https://codeberg.org/akib/emacs-camera")
> +
>   ("caml"		:url "https://github.com/ocaml/caml-mode"
>    :ignored-files ("COPYING")
>    ;; The version 4.7.1 from Melpa-stable seems to correspond to
> -- 
> 2.37.1
>
>
> * GC Buffers - Kill garbage buffers automatically
>
> This kills all garbage buffers (can be obviously customized) after a
> certain amount of idle time.
>
> From 20f25aadddd45112e298c8f4ae07b31ab24161b3 Mon Sep 17 00:00:00 2001
> From: Akib Azmain Turja <akib@disroot.org>
> Date: Mon, 14 Nov 2022 09:56:42 +0600
> Subject: [PATCH] * elpa-packages (gc-buffers): New package
>
> ---
>  elpa-packages | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index b0dd9d3..899fe24 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -185,6 +185,8 @@
>  
>   ("free-keys"           :url "https://github.com/Fuco1/free-keys")
>  
> + ("gc-buffers"          :url "https://codeberg.org/akib/emacs-gc-buffers")
> +
>   ("geiser"		:url "https://gitlab.com/emacs-geiser/geiser.git"
>    :lisp-dir "elisp"
>    :readme "readme.org"
> -- 
> 2.37.1

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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-15 19:57 ` Philip Kaludercic
  2 siblings, 1 reply; 57+ messages in thread
From: Filipp Gunbin @ 2022-11-15 19:53 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

On 14/11/2022 13:42 +0600, Akib Azmain Turja wrote:

[...]
> * GC Buffers - Kill garbage buffers automatically
>
> This kills all garbage buffers (can be obviously customized) after a
> certain amount of idle time.
[...]

Is it in some way superior to clean-buffer-list?



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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-15 19:57 ` Philip Kaludercic
  2022-11-16 12:25   ` Akib Azmain Turja
  2 siblings, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-15 19:57 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

> Before someone asks me to consider assigning my copyright, I would like
> to say that a printed copy of assignment agreement is in my hands right
> now, but I can't sign it now, and I don't know when I'll able to sign
> (don't get me wrong, I really want to sign it).

Do you know when you will know, or are you saying that it isn't worth
waiting?



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-15 19:53 ` Filipp Gunbin
@ 2022-11-16 12:22   ` Akib Azmain Turja
  2022-11-16 16:53     ` Eli Zaretskii
  0 siblings, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-16 12:22 UTC (permalink / raw)
  To: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> On 14/11/2022 13:42 +0600, Akib Azmain Turja wrote:
>
> [...]
>> * GC Buffers - Kill garbage buffers automatically
>>
>> This kills all garbage buffers (can be obviously customized) after a
>> certain amount of idle time.
> [...]
>
> Is it in some way superior to clean-buffer-list?

I'm not sure, since I discovered that command just a few days ago.

However, as I understand reading the manual, it is intended for
file-visited buffers, and not for running every now and then.  But
GC Buffers is mainly targeted at temporary buffers and the buffers
created as a result of various command, for example Flymake diagnostics
buffer, Helpful buffer, etc.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-15 19:57 ` Philip Kaludercic
@ 2022-11-16 12:25   ` Akib Azmain Turja
  2022-11-16 16:07     ` Philip Kaludercic
  2022-11-19 12:05     ` Richard Stallman
  0 siblings, 2 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-16 12:25 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Before someone asks me to consider assigning my copyright, I would like
>> to say that a printed copy of assignment agreement is in my hands right
>> now, but I can't sign it now, and I don't know when I'll able to sign
>> (don't get me wrong, I really want to sign it).
>
> Do you know when you will know, or are you saying that it isn't worth
> waiting?

No, I don't know.  It might be tomorrow (very unlike, but possible), or
even months later.  I think these packages should be published in the
mean time.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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-19 12:05     ` Richard Stallman
  1 sibling, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-16 16:07 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Akib Azmain Turja <akib@disroot.org> writes:
>>
>>> Before someone asks me to consider assigning my copyright, I would like
>>> to say that a printed copy of assignment agreement is in my hands right
>>> now, but I can't sign it now, and I don't know when I'll able to sign
>>> (don't get me wrong, I really want to sign it).
>>
>> Do you know when you will know, or are you saying that it isn't worth
>> waiting?
>
> No, I don't know.  It might be tomorrow (very unlike, but possible), or
> even months later.  I think these packages should be published in the
> mean time.

OK, then adding them to NonGNU ELPA seems like the safer bet.

I'd like to add them, but I'll have to take the time to review them
first, which might take a bit.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 12:22   ` Akib Azmain Turja
@ 2022-11-16 16:53     ` Eli Zaretskii
  2022-11-16 17:43       ` Akib Azmain Turja
  0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2022-11-16 16:53 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: emacs-devel

> From: Akib Azmain Turja <akib@disroot.org>
> Date: Wed, 16 Nov 2022 18:22:58 +0600
> 
> >> * GC Buffers - Kill garbage buffers automatically
> >>
> >> This kills all garbage buffers (can be obviously customized) after a
> >> certain amount of idle time.
> > [...]
> >
> > Is it in some way superior to clean-buffer-list?
> 
> I'm not sure, since I discovered that command just a few days ago.
> 
> However, as I understand reading the manual, it is intended for
> file-visited buffers, and not for running every now and then.  But
> GC Buffers is mainly targeted at temporary buffers and the buffers
> created as a result of various command, for example Flymake diagnostics
> buffer, Helpful buffer, etc.

What about midnight.el -- did you look at that?



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 16:53     ` Eli Zaretskii
@ 2022-11-16 17:43       ` Akib Azmain Turja
  2022-11-16 19:47         ` Eli Zaretskii
  0 siblings, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-16 17:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Akib Azmain Turja <akib@disroot.org>
>> Date: Wed, 16 Nov 2022 18:22:58 +0600
>> 
>> >> * GC Buffers - Kill garbage buffers automatically
>> >>
>> >> This kills all garbage buffers (can be obviously customized) after a
>> >> certain amount of idle time.
>> > [...]
>> >
>> > Is it in some way superior to clean-buffer-list?
>> 
>> I'm not sure, since I discovered that command just a few days ago.
>> 
>> However, as I understand reading the manual, it is intended for
>> file-visited buffers, and not for running every now and then.  But
>> GC Buffers is mainly targeted at temporary buffers and the buffers
>> created as a result of various command, for example Flymake diagnostics
>> buffer, Helpful buffer, etc.
>
> What about midnight.el -- did you look at that?

At a quick look, IIUC midnight.el kills buffers once a day, while
GC Buffers kills buffers after a minute of idle, by default.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 16:07     ` Philip Kaludercic
@ 2022-11-16 17:45       ` Akib Azmain Turja
  2022-11-16 18:19         ` Philip Kaludercic
  0 siblings, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-16 17:45 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>> Akib Azmain Turja <akib@disroot.org> writes:
>>>
>>>> Before someone asks me to consider assigning my copyright, I would like
>>>> to say that a printed copy of assignment agreement is in my hands right
>>>> now, but I can't sign it now, and I don't know when I'll able to sign
>>>> (don't get me wrong, I really want to sign it).
>>>
>>> Do you know when you will know, or are you saying that it isn't worth
>>> waiting?
>>
>> No, I don't know.  It might be tomorrow (very unlike, but possible), or
>> even months later.  I think these packages should be published in the
>> mean time.
>
> OK, then adding them to NonGNU ELPA seems like the safer bet.
>
> I'd like to add them, but I'll have to take the time to review them
> first, which might take a bit.

What do you want to review?  The patches, or the packages?

Anyway, take your time, I'm in no hurry.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 17:45       ` Akib Azmain Turja
@ 2022-11-16 18:19         ` Philip Kaludercic
  2022-11-17 14:28           ` Akib Azmain Turja
  0 siblings, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-16 18:19 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Akib Azmain Turja <akib@disroot.org> writes:
>>
>>> Philip Kaludercic <philipk@posteo.net> writes:
>>>
>>>> Akib Azmain Turja <akib@disroot.org> writes:
>>>>
>>>>> Before someone asks me to consider assigning my copyright, I would like
>>>>> to say that a printed copy of assignment agreement is in my hands right
>>>>> now, but I can't sign it now, and I don't know when I'll able to sign
>>>>> (don't get me wrong, I really want to sign it).
>>>>
>>>> Do you know when you will know, or are you saying that it isn't worth
>>>> waiting?
>>>
>>> No, I don't know.  It might be tomorrow (very unlike, but possible), or
>>> even months later.  I think these packages should be published in the
>>> mean time.
>>
>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>
>> I'd like to add them, but I'll have to take the time to review them
>> first, which might take a bit.
>
> What do you want to review?  The patches, or the packages?

The packages, unless you aren't interested in comments.

> Anyway, take your time, I'm in no hurry.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 17:43       ` Akib Azmain Turja
@ 2022-11-16 19:47         ` Eli Zaretskii
  2022-11-17 14:27           ` Akib Azmain Turja
  0 siblings, 1 reply; 57+ messages in thread
From: Eli Zaretskii @ 2022-11-16 19:47 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: emacs-devel

> From: Akib Azmain Turja <akib@disroot.org>
> Cc: emacs-devel@gnu.org
> Date: Wed, 16 Nov 2022 23:43:33 +0600
> 
> > What about midnight.el -- did you look at that?
> 
> At a quick look, IIUC midnight.el kills buffers once a day, while
> GC Buffers kills buffers after a minute of idle, by default.

And?  If that's the only issue, it shouldn't be hard to extend
midnight.el to do this more frequently, or add a derivative minor mode
for that purpose.

I guess my point is that we should try not to invent new wheels where
old one are available and working.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 19:47         ` Eli Zaretskii
@ 2022-11-17 14:27           ` Akib Azmain Turja
  2022-11-17 18:41             ` Stefan Monnier
                               ` (2 more replies)
  0 siblings, 3 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-17 14:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Akib Azmain Turja <akib@disroot.org>
>> Cc: emacs-devel@gnu.org
>> Date: Wed, 16 Nov 2022 23:43:33 +0600
>> 
>> > What about midnight.el -- did you look at that?
>> 
>> At a quick look, IIUC midnight.el kills buffers once a day, while
>> GC Buffers kills buffers after a minute of idle, by default.
>
> And?  If that's the only issue, it shouldn't be hard to extend
> midnight.el to do this more frequently, or add a derivative minor mode
> for that purpose.

Probably.  Anyway, does midnight.el allow to use a function instead of
regexp?

>
> I guess my point is that we should try not to invent new wheels where
> old one are available and working.
>

Yeah, but I reinvented the wheel six month ago (according to the first
commit) without knowing someone has already invented the wheel before.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 18:19         ` Philip Kaludercic
@ 2022-11-17 14:28           ` Akib Azmain Turja
  2022-11-21 18:32             ` Philip Kaludercic
                               ` (3 more replies)
  0 siblings, 4 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-17 14:28 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>
>>> I'd like to add them, but I'll have to take the time to review them
>>> first, which might take a bit.
>>
>> What do you want to review?  The patches, or the packages?
>
> The packages, unless you aren't interested in comments.

Review if you wish, I welcome feedback.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-17 14:27           ` Akib Azmain Turja
@ 2022-11-17 18:41             ` Stefan Monnier
  2022-11-17 18:51             ` Yuan Fu
  2022-11-17 18:56             ` Eli Zaretskii
  2 siblings, 0 replies; 57+ messages in thread
From: Stefan Monnier @ 2022-11-17 18:41 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Eli Zaretskii, emacs-devel

> Yeah, but I reinvented the wheel six month ago (according to the first
> commit) without knowing someone has already invented the wheel before.

Reinventing is part of the scientific process :-)

In order to ensure real progress, it's important to look at existing wheels
invented independently and see how we can combine their respective ideas
into yet another wheel that's an even better wheel than its ancestors.


        Stefan




^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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
  2 siblings, 1 reply; 57+ messages in thread
From: Yuan Fu @ 2022-11-17 18:51 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Eli Zaretskii, emacs-devel

> 
>> 
>> I guess my point is that we should try not to invent new wheels where
>> old one are available and working.
>> 
> 
> Yeah, but I reinvented the wheel six month ago (according to the first
> commit) without knowing someone has already invented the wheel before.

I believe the point is that we should try to add useful features to existing packages (when technically feasible and favorable) rather than adding new packages.

Yuan


^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-17 14:27           ` Akib Azmain Turja
  2022-11-17 18:41             ` Stefan Monnier
  2022-11-17 18:51             ` Yuan Fu
@ 2022-11-17 18:56             ` Eli Zaretskii
  2 siblings, 0 replies; 57+ messages in thread
From: Eli Zaretskii @ 2022-11-17 18:56 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: emacs-devel

> From: Akib Azmain Turja <akib@disroot.org>
> Cc: emacs-devel@gnu.org
> Date: Thu, 17 Nov 2022 20:27:10 +0600
> 
> >> At a quick look, IIUC midnight.el kills buffers once a day, while
> >> GC Buffers kills buffers after a minute of idle, by default.
> >
> > And?  If that's the only issue, it shouldn't be hard to extend
> > midnight.el to do this more frequently, or add a derivative minor mode
> > for that purpose.
> 
> Probably.  Anyway, does midnight.el allow to use a function instead of
> regexp?

Not that I see, no.  But it should be an easy extension.  Also,
there's a midnight-hook, in which you can do stuff even without such
extensions.

> > I guess my point is that we should try not to invent new wheels where
> > old one are available and working.
> 
> Yeah, but I reinvented the wheel six month ago (according to the first
> commit) without knowing someone has already invented the wheel before.

We are talking about what should be in Emacs and in ELPA.  The fun of
inventing wheels for your own use and joy no one can take from you.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-16 12:25   ` Akib Azmain Turja
  2022-11-16 16:07     ` Philip Kaludercic
@ 2022-11-19 12:05     ` Richard Stallman
  2022-11-19 12:17       ` Philip Kaludercic
  1 sibling, 1 reply; 57+ messages in thread
From: Richard Stallman @ 2022-11-19 12:05 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: philipk, emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >> Before someone asks me to consider assigning my copyright, I would like
  > >> to say that a printed copy of assignment agreement is in my hands right
  > >> now, but I can't sign it now, and I don't know when I'll able to sign
  > >> (don't get me wrong, I really want to sign it).
  > >
  > > Do you know when you will know, or are you saying that it isn't worth
  > > waiting?

  > No, I don't know.  It might be tomorrow (very unlike, but possible), or
  > even months later.  I think these packages should be published in the
  > mean time.

Not in GNU ELPA!  An agreement that MIGHT be signed is not valid.

It is ok to add them to NonGNU ELPA if they meet the other
requirements for NonGNU ELPA.


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)





^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-19 12:05     ` Richard Stallman
@ 2022-11-19 12:17       ` Philip Kaludercic
  0 siblings, 0 replies; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-19 12:17 UTC (permalink / raw)
  To: Richard Stallman; +Cc: Akib Azmain Turja, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
>   > >> Before someone asks me to consider assigning my copyright, I would like
>   > >> to say that a printed copy of assignment agreement is in my hands right
>   > >> now, but I can't sign it now, and I don't know when I'll able to sign
>   > >> (don't get me wrong, I really want to sign it).
>   > >
>   > > Do you know when you will know, or are you saying that it isn't worth
>   > > waiting?
>
>   > No, I don't know.  It might be tomorrow (very unlike, but possible), or
>   > even months later.  I think these packages should be published in the
>   > mean time.
>
> Not in GNU ELPA!  An agreement that MIGHT be signed is not valid.

The initial proposal was to add the packages to NonGNU ELPA, I just
mentioned GNU ELPA to double-check.

> It is ok to add them to NonGNU ELPA if they meet the other
> requirements for NonGNU ELPA.

From what I saw, the requirements are all given.  But I'll try to find
some time tomorrow to check the packages individually.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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-25  7:14             ` Philip Kaludercic
                               ` (2 subsequent siblings)
  3 siblings, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-21 18:32 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>
>>>> I'd like to add them, but I'll have to take the time to review them
>>>> first, which might take a bit.
>>>
>>> What do you want to review?  The patches, or the packages?
>>
>> The packages, unless you aren't interested in comments.
>
> Review if you wish, I welcome feedback.

I've managed to skim through workroom.el.  The code looks great, so I
just have a non-comprehensive list of comments and ideas:


[-- Attachment #2: Type: text/plain, Size: 6559 bytes --]

diff --git a/workroom.el b/workroom.el
index 7091645b85..8bb564e495 100644
--- a/workroom.el
+++ b/workroom.el
@@ -217,12 +217,13 @@ The value is a mode line terminal like `mode-line-format'."
     keymap)
   "Keymap containing all useful commands of Workroom.")
 
-(defvar workroom-mode-map (make-sparse-keymap)
+(defvar workroom-mode-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map workroom-command-map-prefix
+		workroom-command-map)
+    map)
   "Keymap for Workroom mode.")
 
-(define-key workroom-mode-map workroom-command-map-prefix
-            workroom-command-map)
-
 (defun workroom-rebind-command-map-prefix ()
   "Rebind command prefix key sequence `workroom-command-map-prefix'."
   (substitute-key-definition
@@ -234,6 +235,7 @@ The value is a mode line terminal like `mode-line-format'."
 ;;;; Workroom and View Manipulation.
 
 (cl-defstruct (workroom--room
+	       (:predicate workroomp)
                (:constructor workroom--make-room)
                (:copier workroom--copy-room))
   "Structure for workroom."
@@ -253,6 +255,7 @@ The value is a mode line terminal like `mode-line-format'."
    :documentation "`completing-read' history of view names."))
 
 (cl-defstruct (workroom--view
+	       (:predicate workroom-view-p)
                (:constructor workroom--make-view)
                (:copier workroom--copy-view))
   "Structure for view of workroom."
@@ -268,7 +271,7 @@ The value is a mode line terminal like `mode-line-format'."
 (defvar workroom--dont-clear-new-view nil
   "Non-nil mean don't clear empty new views.")
 
-(defvar workroom--rooms nil
+(defvar workroom--rooms nil		;maybe some comments on the structure
   "List of currently live workrooms.")
 
 (defvar workroom-room-history nil
@@ -283,10 +286,6 @@ that.")
 
 (defvar workroom-mode)
 
-(defun workroomp (object)
-  "Return non-nil if OBJECT is a workroom object."
-  (workroom--room-p object))
-
 (defun workroom-name (room)
   "Return the name of workroom ROOM."
   (workroom--room-name room))
@@ -384,10 +383,6 @@ effect, it is not unaltered."
   "Completing read history of view of workroom ROOM."
   (workroom--room-view-history room))
 
-(defun workroom-view-p (object)
-  "Return non-nil if OBJECT is a view object."
-  (workroom--view-p object))
-
 (defun workroom-view-name (view)
   "Return the name of view VIEW."
   (workroom--view-name view))
@@ -435,10 +430,9 @@ A copy is returned, so it can be modified with side-effects."
   "Return the workroom named NAME.
 
 If no such workroom exists, return nil."
-  (catch 'found
-    (dolist (room workroom--rooms nil)
-      (when (string= name (workroom-name room))
-        (throw 'found room)))))
+  (cl-find name workroom--rooms
+	   :key #'workroom-name
+	   :test #'string=))
 
 (defun workroom-get-create (name)
   "Return the workroom named NAME.
@@ -500,7 +494,7 @@ If no such view exists, create a new one named NAME and return that."
     (unless view
       (setq view (workroom--make-view :name name))
       (setf (workroom--room-view-list room)
-            (nconc (workroom--room-view-list room) `(,view))))
+            (nconc (workroom--room-view-list room) (list view))))
     view))
 
 (defun workroom-generate-new-view-name (room name)
@@ -516,7 +510,7 @@ name."
       (let ((n 2))
         (while t
           (let ((str (format "%s<%i>" name n)))
-            (when (not (workroom-view-get room str))
+            (unless (workroom-view-get room str)
               (cl-return str))
             (cl-incf n)))))))
 
@@ -558,7 +552,7 @@ Return DEF when input is empty, where DEF is either a string or nil.
 
 REQUIRE-MATCH and PREDICATE is same as in `completing-read'."
   (completing-read
-   (concat prompt (when def (format " (default %s)" def)) ": ")
+   (concat prompt (and def (format " (default %s)" def)) ": ") ;Compat has `format-prompt'
    (mapcar #'workroom-name workroom--rooms) predicate require-match
    nil 'workroom-room-history def))
 
@@ -670,7 +664,7 @@ If WRITABLE, return a writable object."
 (defun workroom--load-window-config (state)
   "Load window configuration STATE."
   (if state
-      (cl-labels
+      (cl-labels			;perhaps this should be split up?
           ((sanitize (entry)
              (cond
               ;; Do nothing.
@@ -1254,7 +1248,7 @@ ACTION and ARGS are also described there."
   (setf (workroom-buffer-manager-data room)
         (cl-delete-if-not #'buffer-live-p
                           (workroom-buffer-manager-data room)))
-  (pcase action
+  (pcase action				;perhaps match on (cons action args)?
     (:initialize
      (cl-destructuring-bind () args
        (setf (workroom-buffer-manager-data room)
@@ -1507,9 +1501,14 @@ restrict."
 
 (defun workroom--encode-view-1 (view)
   "Encode view VIEW to a writable object."
-  `( :name ,(workroom-view-name view)
-     :window-config ,(workroom-view-window-configuration
-                      view 'writable)))
+  ;; I think it is preferable to define plists with keywords using
+  ;; `list', as all the members of the list can be evaluated during
+  ;; application.
+  (list :name
+	(workroom-view-name view)
+	:window-config
+	(workroom-view-window-configuration
+	 view 'writable)))
 
 (defun workroom--decode-view-1 (object)
   "Decode encoded view OBJECT to a view."
@@ -1589,7 +1588,7 @@ when ROOM was encoded."
 
 (defun workroom-decode-buffer-bookmark (object)
   "Decode OBJECT using `bookmark-jump'."
-  (let* ((buffer nil))
+  (let ((buffer nil))
     (bookmark-jump object (lambda (buf) (setq buffer buf)))
     buffer))
 
@@ -1664,7 +1663,7 @@ any previous bookmark with the same name."
                                      bookmark)))))
     (pcase (plist-get data :version)
       (1
-       (let* ((buffers (cl-delete-if
+       (let ((buffers (cl-delete-if
                         #'null
                         (workroom--decode-buffers
                          (plist-get data :buffers)))))
@@ -1787,6 +1786,7 @@ any previous bookmark with the same name."
   ;; Inject restoring code.
   (when workroom-mode
     (let ((time (format-time-string "%s%N")))
+      ;; I like to use `prin1-to-string' for things like these.
       (insert
        (format
         "
@@ -1951,6 +1951,7 @@ argument while setting as the buffer manager, PROJECT, the project."
 (defun workroom--project-name (project)
   "Return a name for project PROJECT."
   (let ((root (project-root project)))
+    ;; Isn't this `file-name-base' or `directory-file-name'?
     (if (string-match "/\\([^/]+\\)/?\\'" root)
         (match-string 1 root)
       root)))

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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 21:16                 ` Stefan Monnier
  0 siblings, 2 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-22 15:20 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 8906 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>>
>>>>> I'd like to add them, but I'll have to take the time to review them
>>>>> first, which might take a bit.
>>>>
>>>> What do you want to review?  The patches, or the packages?
>>>
>>> The packages, unless you aren't interested in comments.
>>
>> Review if you wish, I welcome feedback.
>
> I've managed to skim through workroom.el.  The code looks great, so I
> just have a non-comprehensive list of comments and ideas:

Really?  It was one of my worst packages until I almost rewrote it over
the last few weeks.

>
> diff --git a/workroom.el b/workroom.el
> index 7091645b85..8bb564e495 100644
> --- a/workroom.el
> +++ b/workroom.el
> @@ -217,12 +217,13 @@ The value is a mode line terminal like `mode-line-format'."
>      keymap)
>    "Keymap containing all useful commands of Workroom.")
>  
> -(defvar workroom-mode-map (make-sparse-keymap)
> +(defvar workroom-mode-map
> +  (let ((map (make-sparse-keymap)))
> +    (define-key map workroom-command-map-prefix
> +		workroom-command-map)
> +    map)
>    "Keymap for Workroom mode.")
>  
> -(define-key workroom-mode-map workroom-command-map-prefix
> -            workroom-command-map)
> -

I just removed that define-key completely.  `workroom-mode` will bind it
when enabled.

>  (defun workroom-rebind-command-map-prefix ()
>    "Rebind command prefix key sequence `workroom-command-map-prefix'."
>    (substitute-key-definition
> @@ -234,6 +235,7 @@ The value is a mode line terminal like `mode-line-format'."
>  ;;;; Workroom and View Manipulation.
>  
>  (cl-defstruct (workroom--room
> +	       (:predicate workroomp)
>                 (:constructor workroom--make-room)
>                 (:copier workroom--copy-room))
>    "Structure for workroom."
> @@ -253,6 +255,7 @@ The value is a mode line terminal like `mode-line-format'."
>     :documentation "`completing-read' history of view names."))
>  
>  (cl-defstruct (workroom--view
> +	       (:predicate workroom-view-p)
>                 (:constructor workroom--make-view)
>                 (:copier workroom--copy-view))
>    "Structure for view of workroom."
> @@ -268,7 +271,7 @@ The value is a mode line terminal like `mode-line-format'."
>  (defvar workroom--dont-clear-new-view nil
>    "Non-nil mean don't clear empty new views.")

They don't get a docstring on my machine.  :(

>  
> -(defvar workroom--rooms nil
> +(defvar workroom--rooms nil		;maybe some comments on the structure
>    "List of currently live workrooms.")

As the docstring describes, it's a list, and all elements satisfies
`workroomp'.

>  
>  (defvar workroom-room-history nil
> @@ -283,10 +286,6 @@ that.")
>  
>  (defvar workroom-mode)
>  
> -(defun workroomp (object)
> -  "Return non-nil if OBJECT is a workroom object."
> -  (workroom--room-p object))
> -
>  (defun workroom-name (room)
>    "Return the name of workroom ROOM."
>    (workroom--room-name room))
> @@ -384,10 +383,6 @@ effect, it is not unaltered."
>    "Completing read history of view of workroom ROOM."
>    (workroom--room-view-history room))
>  
> -(defun workroom-view-p (object)
> -  "Return non-nil if OBJECT is a view object."
> -  (workroom--view-p object))
> -

I just wanted to give it a better docstring.  ;)

>  (defun workroom-view-name (view)
>    "Return the name of view VIEW."
>    (workroom--view-name view))
> @@ -435,10 +430,9 @@ A copy is returned, so it can be modified with side-effects."
>    "Return the workroom named NAME.
>  
>  If no such workroom exists, return nil."
> -  (catch 'found
> -    (dolist (room workroom--rooms nil)
> -      (when (string= name (workroom-name room))
> -        (throw 'found room)))))
> +  (cl-find name workroom--rooms
> +	   :key #'workroom-name
> +	   :test #'string=))

Done, also changed workroom-view-get.

>  
>  (defun workroom-get-create (name)
>    "Return the workroom named NAME.
> @@ -500,7 +494,7 @@ If no such view exists, create a new one named NAME and return that."
>      (unless view
>        (setq view (workroom--make-view :name name))
>        (setf (workroom--room-view-list room)
> -            (nconc (workroom--room-view-list room) `(,view))))
> +            (nconc (workroom--room-view-list room) (list view))))
>      view))

Done.

>  
>  (defun workroom-generate-new-view-name (room name)
> @@ -516,7 +510,7 @@ name."
>        (let ((n 2))
>          (while t
>            (let ((str (format "%s<%i>" name n)))
> -            (when (not (workroom-view-get room str))
> +            (unless (workroom-view-get room str)
>                (cl-return str))
>              (cl-incf n)))))))

I can't believe I wrote (when (not ...) ...)!

>  
> @@ -558,7 +552,7 @@ Return DEF when input is empty, where DEF is either a string or nil.
>  
>  REQUIRE-MATCH and PREDICATE is same as in `completing-read'."
>    (completing-read
> -   (concat prompt (when def (format " (default %s)" def)) ": ")
> +   (concat prompt (and def (format " (default %s)" def)) ": ") ;Compat has `format-prompt'
>     (mapcar #'workroom-name workroom--rooms) predicate require-match
>     nil 'workroom-room-history def))

I don't have much idea about Compat, how does it work?

>  
> @@ -670,7 +664,7 @@ If WRITABLE, return a writable object."
>  (defun workroom--load-window-config (state)
>    "Load window configuration STATE."
>    (if state
> -      (cl-labels
> +      (cl-labels			;perhaps this should be split up?
>            ((sanitize (entry)
>               (cond
>                ;; Do nothing.

Yeah, it's too monolithic.

> @@ -1254,7 +1248,7 @@ ACTION and ARGS are also described there."
>    (setf (workroom-buffer-manager-data room)
>          (cl-delete-if-not #'buffer-live-p
>                            (workroom-buffer-manager-data room)))
> -  (pcase action
> +  (pcase action				;perhaps match on (cons action args)?
>      (:initialize
>       (cl-destructuring-bind () args
>         (setf (workroom-buffer-manager-data room)

I wonder why I used cl-destructuring-bind when there isn't any keyword
arguments.

Fixing...
Fixing...done

> @@ -1507,9 +1501,14 @@ restrict."
>  
>  (defun workroom--encode-view-1 (view)
>    "Encode view VIEW to a writable object."
> -  `( :name ,(workroom-view-name view)
> -     :window-config ,(workroom-view-window-configuration
> -                      view 'writable)))
> +  ;; I think it is preferable to define plists with keywords using
> +  ;; `list', as all the members of the list can be evaluated during
> +  ;; application.
> +  (list :name
> +	(workroom-view-name view)
> +	:window-config
> +	(workroom-view-window-configuration
> +	 view 'writable)))
>  

Done.  Maybe I did too much.

>  (defun workroom--decode-view-1 (object)
>    "Decode encoded view OBJECT to a view."
> @@ -1589,7 +1588,7 @@ when ROOM was encoded."
>  
>  (defun workroom-decode-buffer-bookmark (object)
>    "Decode OBJECT using `bookmark-jump'."
> -  (let* ((buffer nil))
> +  (let ((buffer nil))
>      (bookmark-jump object (lambda (buf) (setq buffer buf)))
>      buffer))
>  
> @@ -1664,7 +1663,7 @@ any previous bookmark with the same name."
>                                       bookmark)))))
>      (pcase (plist-get data :version)
>        (1
> -       (let* ((buffers (cl-delete-if
> +       (let ((buffers (cl-delete-if
>                          #'null
>                          (workroom--decode-buffers
>                           (plist-get data :buffers)))))

Ah, I wasted two bytes for each copy of Workroom worldwide.  :(
Fixed.

> @@ -1787,6 +1786,7 @@ any previous bookmark with the same name."
>    ;; Inject restoring code.
>    (when workroom-mode
>      (let ((time (format-time-string "%s%N")))
> +      ;; I like to use `prin1-to-string' for things like these.
>        (insert
>         (format
>          "

Done, and I got syntax highlighting as a bonus!

> @@ -1951,6 +1951,7 @@ argument while setting as the buffer manager, PROJECT, the project."
>  (defun workroom--project-name (project)
>    "Return a name for project PROJECT."
>    (let ((root (project-root project)))
> +    ;; Isn't this `file-name-base' or `directory-file-name'?
>      (if (string-match "/\\([^/]+\\)/?\\'" root)
>          (match-string 1 root)
>        root)))
>

Yeah, my bad, it's actually `file-name-base'.  I threw the function
away.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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-22 21:16                 ` Stefan Monnier
  1 sibling, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-22 17:07 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Akib Azmain Turja <akib@disroot.org> writes:
>>
>>> Philip Kaludercic <philipk@posteo.net> writes:
>>>
>>>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>>>
>>>>>> I'd like to add them, but I'll have to take the time to review them
>>>>>> first, which might take a bit.
>>>>>
>>>>> What do you want to review?  The patches, or the packages?
>>>>
>>>> The packages, unless you aren't interested in comments.
>>>
>>> Review if you wish, I welcome feedback.
>>
>> I've managed to skim through workroom.el.  The code looks great, so I
>> just have a non-comprehensive list of comments and ideas:
>
> Really?  It was one of my worst packages until I almost rewrote it over
> the last few weeks.

The coding style was clean and the documentation went into details.
That is good stuff in my book :)

>>  (defun workroom-rebind-command-map-prefix ()
>>    "Rebind command prefix key sequence `workroom-command-map-prefix'."
>>    (substitute-key-definition
>> @@ -234,6 +235,7 @@ The value is a mode line terminal like `mode-line-format'."
>>  ;;;; Workroom and View Manipulation.
>>  
>>  (cl-defstruct (workroom--room
>> +	       (:predicate workroomp)
>>                 (:constructor workroom--make-room)
>>                 (:copier workroom--copy-room))
>>    "Structure for workroom."
>> @@ -253,6 +255,7 @@ The value is a mode line terminal like `mode-line-format'."
>>     :documentation "`completing-read' history of view names."))
>>  
>>  (cl-defstruct (workroom--view
>> +	       (:predicate workroom-view-p)
>>                 (:constructor workroom--make-view)
>>                 (:copier workroom--copy-view))
>>    "Structure for view of workroom."
>> @@ -268,7 +271,7 @@ The value is a mode line terminal like `mode-line-format'."
>>  (defvar workroom--dont-clear-new-view nil
>>    "Non-nil mean don't clear empty new views.")
>
> They don't get a docstring on my machine.  :(

That might be the case.  In that case you can keep the previous code,
and perhaps define the prettier variants using `defalias`, or by
manually annotating a function symbol.

>>  
>> -(defvar workroom--rooms nil
>> +(defvar workroom--rooms nil ;maybe some comments on the structure
>>    "List of currently live workrooms.")
>
> As the docstring describes, it's a list, and all elements satisfies
> `workroomp'.

Hmm, I guess that is self-descriptive enough.  I was wondering if this
was a alist or something else.

>> @@ -558,7 +552,7 @@ Return DEF when input is empty, where DEF is either a string or nil.
>>  
>>  REQUIRE-MATCH and PREDICATE is same as in `completing-read'."
>>    (completing-read
>> -   (concat prompt (when def (format " (default %s)" def)) ": ")
>> +   (concat prompt (and def (format " (default %s)" def)) ": ") ;Compat has `format-prompt'
>>     (mapcar #'workroom-name workroom--rooms) predicate require-match
>>     nil 'workroom-room-history def))
>
> I don't have much idea about Compat, how does it work?

Compat is provided on ELPA.  If you add it as a dependency and load it,
it will define missing functionality on older systems.

The documentation goes into greater detail:
https://elpa.gnu.org/packages/doc/compat.html#Usage.

But as everything else, this is just a "fun fact", nothing critical.

>> @@ -1254,7 +1248,7 @@ ACTION and ARGS are also described there."
>>    (setf (workroom-buffer-manager-data room)
>>          (cl-delete-if-not #'buffer-live-p
>>                            (workroom-buffer-manager-data room)))
>> -  (pcase action
>> +  (pcase action				;perhaps match on (cons action args)?
>>      (:initialize
>>       (cl-destructuring-bind () args
>>         (setf (workroom-buffer-manager-data room)
>
> I wonder why I used cl-destructuring-bind when there isn't any keyword
> arguments.
>
> Fixing...
> Fixing...done

I assumed this was just for the sake of consistency?  After all, my
suggestion does do one unnecessary `cons' for the sake of convenience.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-22 17:07                 ` Philip Kaludercic
@ 2022-11-22 17:42                   ` Akib Azmain Turja
  2022-11-25  8:29                     ` Akib Azmain Turja
  0 siblings, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-22 17:42 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 4188 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

>>> I've managed to skim through workroom.el.  The code looks great, so I
>>> just have a non-comprehensive list of comments and ideas:
>>
>> Really?  It was one of my worst packages until I almost rewrote it over
>> the last few weeks.
>
> The coding style was clean and the documentation went into details.
> That is good stuff in my book :)

I can't remember writing any comments, and the docstring are just the
result of checkdoc, which always teases me as I program.

>
>>>  (defun workroom-rebind-command-map-prefix ()
>>>    "Rebind command prefix key sequence `workroom-command-map-prefix'."
>>>    (substitute-key-definition
>>> @@ -234,6 +235,7 @@ The value is a mode line terminal like `mode-line-format'."
>>>  ;;;; Workroom and View Manipulation.
>>>  
>>>  (cl-defstruct (workroom--room
>>> +	       (:predicate workroomp)
>>>                 (:constructor workroom--make-room)
>>>                 (:copier workroom--copy-room))
>>>    "Structure for workroom."
>>> @@ -253,6 +255,7 @@ The value is a mode line terminal like `mode-line-format'."
>>>     :documentation "`completing-read' history of view names."))
>>>  
>>>  (cl-defstruct (workroom--view
>>> +	       (:predicate workroom-view-p)
>>>                 (:constructor workroom--make-view)
>>>                 (:copier workroom--copy-view))
>>>    "Structure for view of workroom."
>>> @@ -268,7 +271,7 @@ The value is a mode line terminal like `mode-line-format'."
>>>  (defvar workroom--dont-clear-new-view nil
>>>    "Non-nil mean don't clear empty new views.")
>>
>> They don't get a docstring on my machine.  :(
>
> That might be the case.  In that case you can keep the previous code,
> and perhaps define the prettier variants using `defalias`, or by
> manually annotating a function symbol.

I think I'll keep the code.

>
>>>  
>>> -(defvar workroom--rooms nil
>>> +(defvar workroom--rooms nil ;maybe some comments on the structure
>>>    "List of currently live workrooms.")
>>
>> As the docstring describes, it's a list, and all elements satisfies
>> `workroomp'.
>
> Hmm, I guess that is self-descriptive enough.  I was wondering if this
> was a alist or something else.
>
>>> @@ -558,7 +552,7 @@ Return DEF when input is empty, where DEF is either a string or nil.
>>>  
>>>  REQUIRE-MATCH and PREDICATE is same as in `completing-read'."
>>>    (completing-read
>>> -   (concat prompt (when def (format " (default %s)" def)) ": ")
>>> +   (concat prompt (and def (format " (default %s)" def)) ": ") ;Compat has `format-prompt'
>>>     (mapcar #'workroom-name workroom--rooms) predicate require-match
>>>     nil 'workroom-room-history def))
>>
>> I don't have much idea about Compat, how does it work?
>
> Compat is provided on ELPA.  If you add it as a dependency and load it,
> it will define missing functionality on older systems.
>
> The documentation goes into greater detail:
> https://elpa.gnu.org/packages/doc/compat.html#Usage.
>
> But as everything else, this is just a "fun fact", nothing critical.

OK, I've change the code to use format-prompt, just only because it's
customizable with the user option minibuffer-default-prompt-format.

>
>>> @@ -1254,7 +1248,7 @@ ACTION and ARGS are also described there."
>>>    (setf (workroom-buffer-manager-data room)
>>>          (cl-delete-if-not #'buffer-live-p
>>>                            (workroom-buffer-manager-data room)))
>>> -  (pcase action
>>> +  (pcase action				;perhaps match on (cons action args)?
>>>      (:initialize
>>>       (cl-destructuring-bind () args
>>>         (setf (workroom-buffer-manager-data room)
>>
>> I wonder why I used cl-destructuring-bind when there isn't any keyword
>> arguments.
>>
>> Fixing...
>> Fixing...done
>
> I assumed this was just for the sake of consistency?  After all, my
> suggestion does do one unnecessary `cons' for the sake of convenience.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-22 15:20               ` Akib Azmain Turja
  2022-11-22 17:07                 ` Philip Kaludercic
@ 2022-11-22 21:16                 ` Stefan Monnier
  1 sibling, 0 replies; 57+ messages in thread
From: Stefan Monnier @ 2022-11-22 21:16 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Philip Kaludercic, Emacs Developer List

>> -(defvar workroom--rooms nil
>> +(defvar workroom--rooms nil		;maybe some comments on the structure
>>    "List of currently live workrooms.")
>
> As the docstring describes, it's a list, and all elements satisfies
> `workroomp'.

I think if you write "List of currently live `workroom--room`s." then
the link should point to the doc of the type.


        Stefan




^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-17 18:51             ` Yuan Fu
@ 2022-11-24 23:38               ` Richard Stallman
  0 siblings, 0 replies; 57+ messages in thread
From: Richard Stallman @ 2022-11-24 23:38 UTC (permalink / raw)
  To: Yuan Fu; +Cc: akib, eliz, emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I believe the point is that we should try to add useful features
  > to existing packages (when technically feasible and favorable)
  > rather than adding new packages.

I agree with that.  The new features will be easier to find
when grouped with related features into a single package.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)





^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-17 14:28           ` Akib Azmain Turja
  2022-11-21 18:32             ` Philip Kaludercic
@ 2022-11-25  7:14             ` Philip Kaludercic
  2022-11-25  7:22               ` Philip Kaludercic
  2022-11-25 13:07               ` Akib Azmain Turja
  2022-11-25 19:07             ` Philip Kaludercic
  2022-11-26 20:07             ` Philip Kaludercic
  3 siblings, 2 replies; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-25  7:14 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>
>>>> I'd like to add them, but I'll have to take the time to review them
>>>> first, which might take a bit.
>>>
>>> What do you want to review?  The patches, or the packages?
>>
>> The packages, unless you aren't interested in comments.
>
> Review if you wish, I welcome feedback.

Sorry for the delay, here are a few more comments:

iwindow:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: comments.diff --]
[-- Type: text/x-diff, Size: 2747 bytes --]

diff --git a/iwindow.el b/iwindow.el
index c808bd26f9..eab2c3084b 100644
--- a/iwindow.el
+++ b/iwindow.el
@@ -46,6 +46,8 @@
 ;;; Code:
 
 (require 'cl-lib)
+;; By adding `seq' as a dependency you could lower the dependency on
+;; the minimum version of Emacs.
 
 (defgroup iwindow nil
   "Interactively manipulate windows."
@@ -54,7 +56,7 @@
   :prefix "iwindow-")
 
 (defcustom iwindow-selection-keys
-  '(?0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9)
+  (number-sequence ?0 ?9)
   "List of keys to use to select window.
 
 Each element should be a key that `read-key' can return."
@@ -141,8 +143,7 @@ list of form (OPTION...), whose length of no more than the length of
       (walk tree nil))
     (run-hook-wrapped 'iwindow-decoration-functions
                       (lambda (fn) (ignore (push fn decorators))))
-    (cl-labels ((call-decorators
-                  (fns)
+    (cl-labels ((call-decorators (fns)
                   (with-selected-window current-window
                     (if fns
                         (funcall (car fns) windows
@@ -157,6 +158,7 @@ list of form (OPTION...), whose length of no more than the length of
 Return the window chosen."
   (if (windowp tree)
       (prog1
+          ;; Is there really a point to using `prog1' here?
           tree
         (redraw-display))
     (let ((option nil)
@@ -192,7 +194,7 @@ WINDOW and ignore WINDOW when PREDICATE returns nil."
                          (seq-filter predicate windows)
                        windows)))
     (when candidates
-      (if (cdr candidates)                 ; (> (length candidates) 1)
+      (if (cdr candidates)                 ;(length> candidates 1)
           (iwindow--ask (iwindow--make-decision-tree
                          (vconcat windows) 0 (length windows)
                          predicate))
@@ -215,7 +217,7 @@ WINDOWS and CALLBACK is described in the docstring of
                                        (alist-get (selected-window)
                                                   ',windows)))
                                  (mapconcat
-                                  (apply-partially #'string ? )
+                                  (apply-partially #'string ?\s)
                                   keys "")
                                ',(alist-get (current-buffer)
                                             original-mode-lines)))))
@@ -261,6 +263,7 @@ WINDOWS and CALLBACK is described in the docstring of
 
 WINDOWS and CALLBACK is described in the docstring of
 `iwindow-decoration-functions', which see."
+  ;; Again, recommending Compat you could make use of `named-let' here
   (cl-labels ((setup-windows (window-list)
                 (with-selected-window (caar window-list)
                   (let ((ov nil))

[-- Attachment #3: Type: text/plain, Size: 8 bytes --]


blow:


[-- Attachment #4: blow.el --]
[-- Type: application/emacs-lisp, Size: 8348 bytes --]

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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
  1 sibling, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-25  7:22 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>>
>>>>> I'd like to add them, but I'll have to take the time to review them
>>>>> first, which might take a bit.
>>>>
>>>> What do you want to review?  The patches, or the packages?
>>>
>>> The packages, unless you aren't interested in comments.
>>
>> Review if you wish, I welcome feedback.
>
> Sorry for the delay, here are a few more comments:

[...]

> blow:

Attached the wrong file here:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: comments.diff --]
[-- Type: text/x-diff, Size: 1117 bytes --]

diff --git a/blow.el b/blow.el
index 15cfd75956..974f745d6b 100644
--- a/blow.el
+++ b/blow.el
@@ -93,7 +93,7 @@
 
 (defun blow--set-mode-list (symbol value)
   "Set SYMBOL's default value to VALUE, SYMBOL should be `blow-mode-list'."
-  (set-default symbol value)
+  (set-default symbol value)		;perhaps `custom-set-default' would be better?
   (when blow-mode
     (blow--setup-all-buffers)))
 
@@ -111,12 +111,12 @@ Don't modify this variable from Lisp programs, use `blow' instead."
                        (sexp :tag "Mode line template")))
   :set #'blow--set-mode-list)
 
-(defvar blow--original-lighters nil
+(defvar blow--original-lighters nil	;why isn't the value initialised?
   "Hash table of modes and their original lighters or nil.")
 
 (defun blow--hash-exists-p (key table)
   "Return t if KEY is in hash table TABLE."
-  (let ((default (make-symbol "blow--nonexistant")))
+  (let ((default (make-symbol "blow--nonexistant"))) ;the prefix is not necessary, the symbol isn't interned anyway
     (not (eq (gethash key table default) default))))
 
 (defun blow--puthash-unless-exists (key value table)

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-22 17:42                   ` Akib Azmain Turja
@ 2022-11-25  8:29                     ` Akib Azmain Turja
  2022-11-25 16:32                       ` Philip Kaludercic
  0 siblings, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-25  8:29 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 4365 bytes --]


Ping?  Any update?

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>>> I've managed to skim through workroom.el.  The code looks great, so I
>>>> just have a non-comprehensive list of comments and ideas:
>>>
>>> Really?  It was one of my worst packages until I almost rewrote it over
>>> the last few weeks.
>>
>> The coding style was clean and the documentation went into details.
>> That is good stuff in my book :)
>
> I can't remember writing any comments, and the docstring are just the
> result of checkdoc, which always teases me as I program.
>
>>
>>>>  (defun workroom-rebind-command-map-prefix ()
>>>>    "Rebind command prefix key sequence `workroom-command-map-prefix'."
>>>>    (substitute-key-definition
>>>> @@ -234,6 +235,7 @@ The value is a mode line terminal like `mode-line-format'."
>>>>  ;;;; Workroom and View Manipulation.
>>>>  
>>>>  (cl-defstruct (workroom--room
>>>> +	       (:predicate workroomp)
>>>>                 (:constructor workroom--make-room)
>>>>                 (:copier workroom--copy-room))
>>>>    "Structure for workroom."
>>>> @@ -253,6 +255,7 @@ The value is a mode line terminal like `mode-line-format'."
>>>>     :documentation "`completing-read' history of view names."))
>>>>  
>>>>  (cl-defstruct (workroom--view
>>>> +	       (:predicate workroom-view-p)
>>>>                 (:constructor workroom--make-view)
>>>>                 (:copier workroom--copy-view))
>>>>    "Structure for view of workroom."
>>>> @@ -268,7 +271,7 @@ The value is a mode line terminal like `mode-line-format'."
>>>>  (defvar workroom--dont-clear-new-view nil
>>>>    "Non-nil mean don't clear empty new views.")
>>>
>>> They don't get a docstring on my machine.  :(
>>
>> That might be the case.  In that case you can keep the previous code,
>> and perhaps define the prettier variants using `defalias`, or by
>> manually annotating a function symbol.
>
> I think I'll keep the code.
>
>>
>>>>  
>>>> -(defvar workroom--rooms nil
>>>> +(defvar workroom--rooms nil ;maybe some comments on the structure
>>>>    "List of currently live workrooms.")
>>>
>>> As the docstring describes, it's a list, and all elements satisfies
>>> `workroomp'.
>>
>> Hmm, I guess that is self-descriptive enough.  I was wondering if this
>> was a alist or something else.
>>
>>>> @@ -558,7 +552,7 @@ Return DEF when input is empty, where DEF is either a string or nil.
>>>>  
>>>>  REQUIRE-MATCH and PREDICATE is same as in `completing-read'."
>>>>    (completing-read
>>>> -   (concat prompt (when def (format " (default %s)" def)) ": ")
>>>> +   (concat prompt (and def (format " (default %s)" def)) ": ") ;Compat has `format-prompt'
>>>>     (mapcar #'workroom-name workroom--rooms) predicate require-match
>>>>     nil 'workroom-room-history def))
>>>
>>> I don't have much idea about Compat, how does it work?
>>
>> Compat is provided on ELPA.  If you add it as a dependency and load it,
>> it will define missing functionality on older systems.
>>
>> The documentation goes into greater detail:
>> https://elpa.gnu.org/packages/doc/compat.html#Usage.
>>
>> But as everything else, this is just a "fun fact", nothing critical.
>
> OK, I've change the code to use format-prompt, just only because it's
> customizable with the user option minibuffer-default-prompt-format.
>
>>
>>>> @@ -1254,7 +1248,7 @@ ACTION and ARGS are also described there."
>>>>    (setf (workroom-buffer-manager-data room)
>>>>          (cl-delete-if-not #'buffer-live-p
>>>>                            (workroom-buffer-manager-data room)))
>>>> -  (pcase action
>>>> +  (pcase action				;perhaps match on (cons action args)?
>>>>      (:initialize
>>>>       (cl-destructuring-bind () args
>>>>         (setf (workroom-buffer-manager-data room)
>>>
>>> I wonder why I used cl-destructuring-bind when there isn't any keyword
>>> arguments.
>>>
>>> Fixing...
>>> Fixing...done
>>
>> I assumed this was just for the sake of consistency?  After all, my
>> suggestion does do one unnecessary `cons' for the sake of convenience.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25  7:22               ` Philip Kaludercic
@ 2022-11-25 12:45                 ` Akib Azmain Turja
  0 siblings, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-25 12:45 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:


[...]

>> blow:
>
> Attached the wrong file here:
>
> diff --git a/blow.el b/blow.el
> index 15cfd75956..974f745d6b 100644
> --- a/blow.el
> +++ b/blow.el
> @@ -93,7 +93,7 @@
>  
>  (defun blow--set-mode-list (symbol value)
>    "Set SYMBOL's default value to VALUE, SYMBOL should be `blow-mode-list'."
> -  (set-default symbol value)
> +  (set-default symbol value)		;perhaps `custom-set-default' would be better?
>    (when blow-mode
>      (blow--setup-all-buffers)))
>  

Thanks, and done.

> @@ -111,12 +111,12 @@ Don't modify this variable from Lisp programs, use `blow' instead."
>                         (sexp :tag "Mode line template")))
>    :set #'blow--set-mode-list)
>  
> -(defvar blow--original-lighters nil
> +(defvar blow--original-lighters nil	;why isn't the value initialised?
>    "Hash table of modes and their original lighters or nil.")
>  

I don't know.  Anyway, I change to initialize to (make-hash-table).

>  (defun blow--hash-exists-p (key table)
>    "Return t if KEY is in hash table TABLE."
> -  (let ((default (make-symbol "blow--nonexistant")))
> +  (let ((default (make-symbol "blow--nonexistant"))) ;the prefix is not necessary, the symbol isn't interned anyway
>      (not (eq (gethash key table default) default))))
>  
>  (defun blow--puthash-unless-exists (key value table)
>

Removed.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25  7:14             ` Philip Kaludercic
  2022-11-25  7:22               ` Philip Kaludercic
@ 2022-11-25 13:07               ` Akib Azmain Turja
  2022-11-25 17:16                 ` Akib Azmain Turja
  2022-11-25 17:31                 ` Philip Kaludercic
  1 sibling, 2 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-25 13:07 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 4219 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

>
> iwindow:
>
> diff --git a/iwindow.el b/iwindow.el
> index c808bd26f9..eab2c3084b 100644
> --- a/iwindow.el
> +++ b/iwindow.el
> @@ -46,6 +46,8 @@
>  ;;; Code:
>  
>  (require 'cl-lib)
> +;; By adding `seq' as a dependency you could lower the dependency on
> +;; the minimum version of Emacs.
>  

Thanks.  But then I would need to do the following:

+ cl-mapcar -> seq-mapn
+ cl-labels -> named-let (right?)
+ cl-letf* -> What?  unwind-protect?

Suggestions appreciated.

>  (defgroup iwindow nil
>    "Interactively manipulate windows."
> @@ -54,7 +56,7 @@
>    :prefix "iwindow-")
>  
>  (defcustom iwindow-selection-keys
> -  '(?0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9)
> +  (number-sequence ?0 ?9)
>    "List of keys to use to select window.
>  
>  Each element should be a key that `read-key' can return."

Done.

> @@ -141,8 +143,7 @@ list of form (OPTION...), whose length of no more than the length of
>        (walk tree nil))
>      (run-hook-wrapped 'iwindow-decoration-functions
>                        (lambda (fn) (ignore (push fn decorators))))
> -    (cl-labels ((call-decorators
> -                  (fns)
> +    (cl-labels ((call-decorators (fns)
>                    (with-selected-window current-window
>                      (if fns
>                          (funcall (car fns) windows

Done.  IIRC, Emacs indented the code as the following when I wrote it:

--8<---------------cut here---------------start------------->8---
    (cl-labels ((call-decorators (fns)
                                 (with-selected-window current-window
                                   (if fns
                                       (funcall (car fns) windows
--8<---------------cut here---------------end--------------->8---

So I placed the argument list on its own line.


> @@ -157,6 +158,7 @@ list of form (OPTION...), whose length of no more than the length of
>  Return the window chosen."
>    (if (windowp tree)
>        (prog1
> +          ;; Is there really a point to using `prog1' here?
>            tree
>          (redraw-display))
>      (let ((option nil)

I don't think so.  I don't know why I wrote that, but looks like the
code is same as (progn (redraw-display) tree).  Changed.

> @@ -192,7 +194,7 @@ WINDOW and ignore WINDOW when PREDICATE returns nil."
>                           (seq-filter predicate windows)
>                         windows)))
>      (when candidates
> -      (if (cdr candidates)                 ; (> (length candidates) 1)
> +      (if (cdr candidates)                 ;(length> candidates 1)
>            (iwindow--ask (iwindow--make-decision-tree
>                           (vconcat windows) 0 (length windows)
>                           predicate))

I was using Emacs 27 back then, hence that comment.  Now I changed that.
Anyway, is there any benefit that new primitive?

> @@ -215,7 +217,7 @@ WINDOWS and CALLBACK is described in the docstring of
>                                         (alist-get (selected-window)
>                                                    ',windows)))
>                                   (mapconcat
> -                                  (apply-partially #'string ? )
> +                                  (apply-partially #'string ?\s)
>                                    keys "")
>                                 ',(alist-get (current-buffer)
>                                              original-mode-lines)))))

Nice idea.  Done.

> @@ -261,6 +263,7 @@ WINDOWS and CALLBACK is described in the docstring of
>  
>  WINDOWS and CALLBACK is described in the docstring of
>  `iwindow-decoration-functions', which see."
> +  ;; Again, recommending Compat you could make use of `named-let' here
>    (cl-labels ((setup-windows (window-list)
>                  (with-selected-window (caar window-list)
>                    (let ((ov nil))

Hmm, tail call optimization, interesting.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25  8:29                     ` Akib Azmain Turja
@ 2022-11-25 16:32                       ` Philip Kaludercic
  2022-11-25 17:14                         ` Akib Azmain Turja
  0 siblings, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-25 16:32 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

> Ping?  Any update?

Sorry, I was planning to review the other packages before pushing the
commits.  The only reason I didn't push anything yet is because I have
all the commits on a single branch. 



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25 16:32                       ` Philip Kaludercic
@ 2022-11-25 17:14                         ` Akib Azmain Turja
  0 siblings, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-25 17:14 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Ping?  Any update?
>
> Sorry, I was planning to review the other packages before pushing the
> commits.  The only reason I didn't push anything yet is because I have
> all the commits on a single branch.

That's OK, take your time.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25 13:07               ` Akib Azmain Turja
@ 2022-11-25 17:16                 ` Akib Azmain Turja
  2022-11-25 17:31                 ` Philip Kaludercic
  1 sibling, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-25 17:16 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> diff --git a/iwindow.el b/iwindow.el
>> index c808bd26f9..eab2c3084b 100644
>> --- a/iwindow.el
>> +++ b/iwindow.el
>> @@ -46,6 +46,8 @@
>>  ;;; Code:
>>  
>>  (require 'cl-lib)
>> +;; By adding `seq' as a dependency you could lower the dependency on
>> +;; the minimum version of Emacs.
>>  
>
> Thanks.  But then I would need to do the following:
>
> + cl-mapcar -> seq-mapn
> + cl-labels -> named-let (right?)
> + cl-letf* -> What?  unwind-protect?
>
> Suggestions appreciated.

I did some changes, and now IWindow is (hopefully) compatible with Emacs
24.3.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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
  1 sibling, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-25 17:31 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>
>> iwindow:
>>
>> diff --git a/iwindow.el b/iwindow.el
>> index c808bd26f9..eab2c3084b 100644
>> --- a/iwindow.el
>> +++ b/iwindow.el
>> @@ -46,6 +46,8 @@
>>  ;;; Code:
>>  
>>  (require 'cl-lib)
>> +;; By adding `seq' as a dependency you could lower the dependency on
>> +;; the minimum version of Emacs.
>>  
>
> Thanks.  But then I would need to do the following:
>
> + cl-mapcar -> seq-mapn
> + cl-labels -> named-let (right?)
> + cl-letf* -> What?  unwind-protect?

Eh, I didn't mean to say that you should replace cl-lib with seq, you
can use both.

> Suggestions appreciated.
>
>>  (defgroup iwindow nil
>>    "Interactively manipulate windows."
>> @@ -54,7 +56,7 @@
>>    :prefix "iwindow-")
>>  
>>  (defcustom iwindow-selection-keys
>> -  '(?0 ?1 ?2 ?3 ?4 ?5 ?6 ?7 ?8 ?9)
>> +  (number-sequence ?0 ?9)
>>    "List of keys to use to select window.
>>  
>>  Each element should be a key that `read-key' can return."
>
> Done.
>
>> @@ -141,8 +143,7 @@ list of form (OPTION...), whose length of no more than the length of
>>        (walk tree nil))
>>      (run-hook-wrapped 'iwindow-decoration-functions
>>                        (lambda (fn) (ignore (push fn decorators))))
>> -    (cl-labels ((call-decorators
>> -                  (fns)
>> +    (cl-labels ((call-decorators (fns)
>>                    (with-selected-window current-window
>>                      (if fns
>>                          (funcall (car fns) windows
>
> Done.  IIRC, Emacs indented the code as the following when I wrote it:
>
>     (cl-labels ((call-decorators (fns)
>                                  (with-selected-window current-window
>                                    (if fns
>                                        (funcall (car fns) windows
>
> So I placed the argument list on its own line.

Perhaps this was changed recently, because I explicitly tried
re-indenting the function and it looked fine.

>
>> @@ -157,6 +158,7 @@ list of form (OPTION...), whose length of no more than the length of
>>  Return the window chosen."
>>    (if (windowp tree)
>>        (prog1
>> +          ;; Is there really a point to using `prog1' here?
>>            tree
>>          (redraw-display))
>>      (let ((option nil)
>
> I don't think so.  I don't know why I wrote that, but looks like the
> code is same as (progn (redraw-display) tree).  Changed.
>
>> @@ -192,7 +194,7 @@ WINDOW and ignore WINDOW when PREDICATE returns nil."
>>                           (seq-filter predicate windows)
>>                         windows)))
>>      (when candidates
>> -      (if (cdr candidates)                 ; (> (length candidates) 1)
>> +      (if (cdr candidates)                 ;(length> candidates 1)
>>            (iwindow--ask (iwindow--make-decision-tree
>>                           (vconcat windows) 0 (length windows)
>>                           predicate))
>
> I was using Emacs 27 back then, hence that comment.  Now I changed that.
> Anyway, is there any benefit that new primitive?

This was probably the most pointless input I had.  The only advantage
that (length> candidates 1) has over (cdr candidates) is that it makes
your intention more explicit.

>> @@ -215,7 +217,7 @@ WINDOWS and CALLBACK is described in the docstring of
>>                                         (alist-get (selected-window)
>>                                                    ',windows)))
>>                                   (mapconcat
>> -                                  (apply-partially #'string ? )
>> +                                  (apply-partially #'string ?\s)
>>                                    keys "")
>>                                 ',(alist-get (current-buffer)
>>                                              original-mode-lines)))))
>
> Nice idea.  Done.
>
>> @@ -261,6 +263,7 @@ WINDOWS and CALLBACK is described in the docstring of
>>  
>>  WINDOWS and CALLBACK is described in the docstring of
>>  `iwindow-decoration-functions', which see."
>> +  ;; Again, recommending Compat you could make use of `named-let' here
>>    (cl-labels ((setup-windows (window-list)
>>                  (with-selected-window (caar window-list)
>>                    (let ((ov nil))
>
> Hmm, tail call optimization, interesting.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-17 14:28           ` Akib Azmain Turja
  2022-11-21 18:32             ` Philip Kaludercic
  2022-11-25  7:14             ` Philip Kaludercic
@ 2022-11-25 19:07             ` Philip Kaludercic
  2022-11-26  7:49               ` Akib Azmain Turja
  2022-11-26 18:44               ` Akib Azmain Turja
  2022-11-26 20:07             ` Philip Kaludercic
  3 siblings, 2 replies; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-25 19:07 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>
>>>> I'd like to add them, but I'll have to take the time to review them
>>>> first, which might take a bit.
>>>
>>> What do you want to review?  The patches, or the packages?
>>
>> The packages, unless you aren't interested in comments.
>
> Review if you wish, I welcome feedback.

Few more comments:

gnu-indent:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: comments.diff --]
[-- Type: text/x-diff, Size: 2890 bytes --]

diff --git a/gnu-indent.el b/gnu-indent.el
index 1a37851e96..8aea4161dc 100644
--- a/gnu-indent.el
+++ b/gnu-indent.el
@@ -51,18 +51,17 @@
 ;; Autoload so that users can set it as file local variable without
 ;; warning.
 ;;;###autoload
-(progn
-  (defcustom gnu-indent-options nil
-    "Arguments to pass to GNU Indent."
-    :type '(repeat string)
-    :safe (lambda (val)
-            (let ((valid t))
-              (while (and valid val)
-                (unless (stringp (car val))
-                  (setq valid nil))
-                (setq val (cdr val)))
-              valid))
-    :group 'gnu-indent))
+(defcustom gnu-indent-options nil
+  "Arguments to pass to GNU Indent."
+  :type '(repeat string)
+  :safe (lambda (val)
+          (let ((valid t))
+            (while (and valid val)
+              (unless (stringp (car val))
+                (setq valid nil))
+              (setq val (cdr val)))
+            valid))
+  :group 'gnu-indent)
 
 ;;;###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))
             (unless (eq (process-exit-status process) 0)
-              (display-buffer (process-buffer process))
+              (pop-to-buffer (process-buffer process))
               (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))))
         (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")))))
+
 ;;;###autoload
 (define-minor-mode gnu-indent-mode
   "Indent buffer automatically with GNU Indent."
   :lighter " GNU-Indent"
-  :keymap nil
   (if gnu-indent-mode
       (add-hook 'before-save-hook #'gnu-indent-buffer nil t)
     (remove-hook 'before-save-hook #'gnu-indent-buffer t)))

[-- Attachment #3: Type: text/plain, Size: 11 bytes --]


devhelp:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: comments.diff --]
[-- Type: text/x-diff, Size: 2135 bytes --]

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?
+
 ;; You can also bookmark pages, with the standard `bookmark-set' function.
 
 ;;; Code:
@@ -89,7 +93,7 @@ Integer means use that many columns.  Nil means use full window width."
 
 Note that on GNU Guix, Nix or other FHS (Filesystem Hierarchy Standard)
 non-compliant distributions, the default value won't work.  For GNU Guix,
-set it to '(\"/run/current-system/profile/share/doc/\"
+set it to \\='(\"/run/current-system/profile/share/doc/\"
 \"/run/current-system/profile/share/gtk-doc/html/\"
 \"~/.guix-profile/share/doc/\" \"~/.guix-profile/share/gtk-doc/html/\")."
   :type '(repeat directory))
@@ -147,6 +151,7 @@ absolute path to it."
                    ,(expand-file-name (dom-attr sec 'link) base)
                    ,(mapcar #'process-section (children-by-tag
                                                sec 'sub)))))
+      ;; `List' would be better here, right?
       `(,(or (dom-attr dom 'title) "Untitled")
         ,(or (dom-attr dom 'name) (file-name-base file))
         ,(or (dom-attr dom 'language) "any")
@@ -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
@@ -447,6 +453,8 @@ When BASE is given, use it to make relative URLs absolute."
     (recenter)))
 
 (defun devhelp-history-back (&optional n)
+  ;; Is the variable actually optional, or won't the + raise a wrong
+  ;; type signal if invoked without an argument?
   "Go to the previous page.
 
 When prefix argument N is given, go to Nth previous page."

[-- Attachment #5: Type: text/plain, Size: 12 bytes --]


why-this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: comments.diff --]
[-- Type: text/x-diff, Size: 4249 bytes --]

diff --git a/why-this.el b/why-this.el
index 287ee99921..19acb90f9f 100644
--- a/why-this.el
+++ b/why-this.el
@@ -36,6 +36,10 @@
 (require 'timezone)
 (require 'color)
 
+;; A very high-level request that would probably take too much work to
+;; actualise, would be to figure out some way to integrate this into
+;; VC.  That way you can make use of this even if you don't use Git or Hg.
+
 (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)
@@ -326,6 +331,7 @@ TIME-FORMAT is used to format data."
              (?t . (why-this-format-time
                     time-format (plist-get data :time)))
              (?i . (plist-get data :desc)))))
+      ;; `format-spec' should make this simpler
       (replace-regexp-in-string
        "%."
        (lambda (str)
@@ -342,7 +348,7 @@ TIME-FORMAT is used to format data."
 (defun why-this--overlay-bg-type (pos)
   "Return the background type for overlay at POS."
   (cond
-   ((and (use-region-p)
+   ((and (use-region-p)                         ;perhaps `region-active-p'?
          (>= pos (region-beginning))
          (< pos (region-end)))
     'region)
@@ -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
@@ -382,12 +389,10 @@ TIME-FORMAT is used to format data."
   (while why-this--overlays
     (delete-overlay (pop why-this--overlays)))
   (when why-this-mode
-    (let* ((begin (line-number-at-pos (if (use-region-p)
-                                          (region-beginning)
-                                        (point))))
-           (end (1+ (line-number-at-pos (if (use-region-p)
-                                            (region-end)
-                                          (point)))))
+    (let* ((line (line-number-at-pos (if (use-region-p)
+                                         (region-beginning)
+                                       (point))))
+           (begin line) (end (1+ line))
            (backend why-this--backend)
            (data (funcall backend 'line-data begin end)))
       (dolist (i (number-sequence 0 (- end begin 1)))
@@ -446,6 +451,8 @@ TIME-FORMAT is used to format data."
                                          (region-end)
                                        (point))))))
     (setq
+     ;; Perhaps this could be made more readble by using
+     ;; `thread-last' or the above let binding
      why-this--overlays
      (delq
       nil
@@ -458,10 +465,10 @@ TIME-FORMAT is used to format data."
                          (< line end)
                          (eq line (overlay-get ov 'why-this-line)))))
              (progn
-               (let* ((ov-start (overlay-start ov))
-                      line-begin
-                      line-end
-                      column)
+               (let ((ov-start (overlay-start ov))
+                     line-begin
+                     line-end
+                     column)
                  (save-excursion
                    (goto-char ov-start)
                    (setq line-begin (line-beginning-position))
@@ -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)

[-- Attachment #7: Type: text/plain, Size: 288 bytes --]


A general comment might be that adding a .dir-locals.el file that
regulates Emcas' whitespace behaviour would be nice to have.

Also, I don't think you need to distribute the texinfo.tex files with
your packages, or at the least you could mark them as ignorable in the
.elpaignore file.

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25 17:31                 ` Philip Kaludercic
@ 2022-11-26  5:53                   ` Akib Azmain Turja
  2022-11-26 20:12                     ` Stefan Monnier
  0 siblings, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-26  5:53 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>>
>>> iwindow:
>>>
>>> diff --git a/iwindow.el b/iwindow.el
>>> index c808bd26f9..eab2c3084b 100644
>>> --- a/iwindow.el
>>> +++ b/iwindow.el
>>> @@ -46,6 +46,8 @@
>>>  ;;; Code:
>>>  
>>>  (require 'cl-lib)
>>> +;; By adding `seq' as a dependency you could lower the dependency on
>>> +;; the minimum version of Emacs.
>>>  
>>
>> Thanks.  But then I would need to do the following:
>>
>> + cl-mapcar -> seq-mapn
>> + cl-labels -> named-let (right?)
>> + cl-letf* -> What?  unwind-protect?
>
> Eh, I didn't mean to say that you should replace cl-lib with seq, you
> can use both.

But I have already did it!  Anyway, the named-let code is much more
readable to me, and seq-mapn and cl-mapcar are basically the same.  And
I replaced cl-letf* with unwind-protect (oops), but seems like I
optimized the code at least little bit.  ;)

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25 19:07             ` Philip Kaludercic
@ 2022-11-26  7:49               ` Akib Azmain Turja
  2022-11-27  8:11                 ` Philip Kaludercic
  2022-11-26 18:44               ` Akib Azmain Turja
  1 sibling, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-26  7:49 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 11840 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

[...]

>
> gnu-indent:
>
> diff --git a/gnu-indent.el b/gnu-indent.el
> index 1a37851e96..8aea4161dc 100644
> --- a/gnu-indent.el
> +++ b/gnu-indent.el
> @@ -51,18 +51,17 @@
>  ;; Autoload so that users can set it as file local variable without
>  ;; warning.
>  ;;;###autoload
> -(progn
> -  (defcustom gnu-indent-options nil
> -    "Arguments to pass to GNU Indent."
> -    :type '(repeat string)
> -    :safe (lambda (val)
> -            (let ((valid t))
> -              (while (and valid val)
> -                (unless (stringp (car val))
> -                  (setq valid nil))
> -                (setq val (cdr val)))
> -              valid))
> -    :group 'gnu-indent))
> +(defcustom gnu-indent-options nil
> +  "Arguments to pass to GNU Indent."
> +  :type '(repeat string)
> +  :safe (lambda (val)
> +          (let ((valid t))
> +            (while (and valid val)
> +              (unless (stringp (car val))
> +                (setq valid nil))
> +              (setq val (cdr val)))
> +            valid))
> +  :group 'gnu-indent)

Done.

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

>              (unless (eq (process-exit-status process) 0)
> -              (display-buffer (process-buffer process))
> +              (pop-to-buffer (process-buffer process))

Done.

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

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

>  ;;;###autoload
>  (define-minor-mode gnu-indent-mode
>    "Indent buffer automatically with GNU Indent."
>    :lighter " GNU-Indent"
> -  :keymap nil
>    (if gnu-indent-mode
>        (add-hook 'before-save-hook #'gnu-indent-buffer nil t)
>      (remove-hook 'before-save-hook #'gnu-indent-buffer t)))

Done.

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

>  ;; You can also bookmark pages, with the standard `bookmark-set' function.
>
>  ;;; Code:
> @@ -89,7 +93,7 @@ Integer means use that many columns.  Nil means use full window width."
>
>  Note that on GNU Guix, Nix or other FHS (Filesystem Hierarchy Standard)
>  non-compliant distributions, the default value won't work.  For GNU Guix,
> -set it to '(\"/run/current-system/profile/share/doc/\"
> +set it to \\='(\"/run/current-system/profile/share/doc/\"

Done.

>  \"/run/current-system/profile/share/gtk-doc/html/\"
>  \"~/.guix-profile/share/doc/\" \"~/.guix-profile/share/gtk-doc/html/\")."
>    :type '(repeat directory))
> @@ -147,6 +151,7 @@ absolute path to it."
>                     ,(expand-file-name (dom-attr sec 'link) base)
>                     ,(mapcar #'process-section (children-by-tag
>                                                 sec 'sub)))))
> +      ;; `List' would be better here, right?
>        `(,(or (dom-attr dom 'title) "Untitled")
>          ,(or (dom-attr dom 'name) (file-name-base file))
>          ,(or (dom-attr dom 'language) "any")

Yes.  Change done.

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

> @@ -447,6 +453,8 @@ When BASE is given, use it to make relative URLs absolute."
>      (recenter)))
>
>  (defun devhelp-history-back (&optional n)
> +  ;; Is the variable actually optional, or won't the + raise a wrong
> +  ;; type signal if invoked without an argument?
>    "Go to the previous page.
>
>  When prefix argument N is given, go to Nth previous page."

Oh, yes, it would; now fixed.

>
>
> why-this:
>
> diff --git a/why-this.el b/why-this.el
> index 287ee99921..19acb90f9f 100644
> --- a/why-this.el
> +++ b/why-this.el
> @@ -36,6 +36,10 @@
>  (require 'timezone)
>  (require 'color)
>
> +;; A very high-level request that would probably take too much work to
> +;; actualise, would be to figure out some way to integrate this into
> +;; VC.  That way you can make use of this even if you don't use Git or Hg.
> +

Exactly, using VC would be a lot better idea.  I just use Git, and the
Hg backend is just a proof-of-concept that Why-This can support other
VCs.

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

> @@ -326,6 +331,7 @@ TIME-FORMAT is used to format data."
>               (?t . (why-this-format-time
>                      time-format (plist-get data :time)))
>               (?i . (plist-get data :desc)))))
> +      ;; `format-spec' should make this simpler
>        (replace-regexp-in-string
>         "%."
>         (lambda (str)

Thanks, I just learned something new.  format-spec made it a lot easier.

> @@ -342,7 +348,7 @@ TIME-FORMAT is used to format data."
>  (defun why-this--overlay-bg-type (pos)
>    "Return the background type for overlay at POS."
>    (cond
> -   ((and (use-region-p)
> +   ((and (use-region-p)                         ;perhaps `region-active-p'?
>           (>= pos (region-beginning))
>           (< pos (region-end)))
>      'region)

Hmm, it doesn't matter, both works.  I changed to use region-active-p.

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

> @@ -382,12 +389,10 @@ TIME-FORMAT is used to format data."
>    (while why-this--overlays
>      (delete-overlay (pop why-this--overlays)))
>    (when why-this-mode
> -    (let* ((begin (line-number-at-pos (if (use-region-p)
> -                                          (region-beginning)
> -                                        (point))))
> -           (end (1+ (line-number-at-pos (if (use-region-p)
> -                                            (region-end)
> -                                          (point)))))
> +    (let* ((line (line-number-at-pos (if (use-region-p)
> +                                         (region-beginning)
> +                                       (point))))
> +           (begin line) (end (1+ line))
>             (backend why-this--backend)
>             (data (funcall backend 'line-data begin end)))
>        (dolist (i (number-sequence 0 (- end begin 1)))

Thanks.

> @@ -446,6 +451,8 @@ TIME-FORMAT is used to format data."
>                                           (region-end)
>                                         (point))))))
>      (setq
> +     ;; Perhaps this could be made more readble by using
> +     ;; `thread-last' or the above let binding
>       why-this--overlays
>       (delq
>        nil

Thanks, just learned another thing.  Changed to use thread-last.

> @@ -458,10 +465,10 @@ TIME-FORMAT is used to format data."
>                           (< line end)
>                           (eq line (overlay-get ov 'why-this-line)))))
>               (progn
> -               (let* ((ov-start (overlay-start ov))
> -                      line-begin
> -                      line-end
> -                      column)
> +               (let ((ov-start (overlay-start ov))
> +                     line-begin
> +                     line-end
> +                     column)
>                   (save-excursion
>                     (goto-char ov-start)
>                     (setq line-begin (line-beginning-position))

Done.

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

>
>
> A general comment might be that adding a .dir-locals.el file that
> regulates Emcas' whitespace behaviour would be nice to have.
>

Thanks.  I'll add.

> Also, I don't think you need to distribute the texinfo.tex files with
> your packages, or at the least you could mark them as ignorable in the
> .elpaignore file.
>

That's there for someone to build the PDF manuals.  I'll add them to
.elpaignore, since they aren't needed for Info manual.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-25 19:07             ` Philip Kaludercic
  2022-11-26  7:49               ` Akib Azmain Turja
@ 2022-11-26 18:44               ` Akib Azmain Turja
  1 sibling, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-26 18:44 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

[...]

> A general comment might be that adding a .dir-locals.el file that
> regulates Emcas' whitespace behaviour would be nice to have.
>
> Also, I don't think you need to distribute the texinfo.tex files with
> your packages, or at the least you could mark them as ignorable in the
> .elpaignore file.
>

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-17 14:28           ` Akib Azmain Turja
                               ` (2 preceding siblings ...)
  2022-11-25 19:07             ` Philip Kaludercic
@ 2022-11-26 20:07             ` Philip Kaludercic
  2022-11-26 20:17               ` Philip Kaludercic
  2022-11-27  6:40               ` [NonGNU ELPA] 11 " Akib Azmain Turja
  3 siblings, 2 replies; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-26 20:07 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>
>>>> I'd like to add them, but I'll have to take the time to review them
>>>> first, which might take a bit.
>>>
>>> What do you want to review?  The patches, or the packages?
>>
>> The packages, unless you aren't interested in comments.
>
> Review if you wish, I welcome feedback.

One minor one more:


[-- Attachment #2: comments.diff --]
[-- Type: text/x-diff, Size: 844 bytes --]

diff --git a/minibar.el b/minibar.el
index 983b3ff58b..0eb6fd338d 100644
--- a/minibar.el
+++ b/minibar.el
@@ -137,9 +137,7 @@ a string to display, or nil in case there is to show."
 ;;;###autoload
 (define-minor-mode minibar-mode
   "Toggle Minibar display."
-  :init-value nil
   :lighter " Minibar"
-  :keymap nil
   :global t
   (if minibar-mode
       (progn
@@ -450,6 +448,9 @@ when it was recorded.")
                (lambda (i)
                  (let ((load (minibar--module-cpu-calculate-load
                               (format "cpu%i" i))))
+		   ;; You should be able to simplify this with an
+		   ;; `alist-get' call and something like #'> as the
+		   ;; equality test.
                    (cond
                     ((and (char-displayable-p ?█)  ; #x2588
                           (>= load 87.5))

[-- Attachment #3: Type: text/plain, Size: 62 bytes --]


The rest looks fine, so I'll push the commits to nongnu.git.

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-26  5:53                   ` Akib Azmain Turja
@ 2022-11-26 20:12                     ` Stefan Monnier
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Monnier @ 2022-11-26 20:12 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Philip Kaludercic, Emacs Developer List

> Anyway, the named-let code is much more readable to me,

Music to my ears,


        Stefan




^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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  6:40               ` [NonGNU ELPA] 11 " Akib Azmain Turja
  1 sibling, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-26 20:17 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>>
>>>>> I'd like to add them, but I'll have to take the time to review them
>>>>> first, which might take a bit.
>>>>
>>>> What do you want to review?  The patches, or the packages?
>>>
>>> The packages, unless you aren't interested in comments.
>>
>> Review if you wish, I welcome feedback.
>
> One minor one more:
>
>
>
> The rest looks fine, so I'll push the commits to nongnu.git.

I just realised that none of the patches can be applies, because the
last commit in nongnu.git changed the package names from strings to
symbols.  This makes applying the patches a nuisance.  Could you perhaps
update the patches to use symbol for names, and make sure nothing has
changed?



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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 18:40                   ` [NonGNU ELPA] 12 " Akib Azmain Turja
  0 siblings, 2 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-26 21:12 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

[...]

> I just realised that none of the patches can be applies, because the
> last commit in nongnu.git changed the package names from strings to
> symbols.  This makes applying the patches a nuisance.  Could you perhaps
> update the patches to use symbol for names, and make sure nothing has
> changed?
>

Hmm, I think it's a good decision to use symbols.

It's already midnight here, so I'll update the patches tomorrow.  BTW,
can you please you review my Eat package[1] (terminal emulator, you have
probably noticed it)?  I'm going to submit that too with the next batch
of patches.

Bye, going to sleep.  Good night!

Footnotes:
[1]  https://codeberg.org/akib/emacs-eat/

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-26 20:07             ` Philip Kaludercic
  2022-11-26 20:17               ` Philip Kaludercic
@ 2022-11-27  6:40               ` Akib Azmain Turja
  1 sibling, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-27  6:40 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>>>> OK, then adding them to NonGNU ELPA seems like the safer bet.
>>>>>
>>>>> I'd like to add them, but I'll have to take the time to review them
>>>>> first, which might take a bit.
>>>>
>>>> What do you want to review?  The patches, or the packages?
>>>
>>> The packages, unless you aren't interested in comments.
>>
>> Review if you wish, I welcome feedback.
>
> One minor one more:
>
> diff --git a/minibar.el b/minibar.el
> index 983b3ff58b..0eb6fd338d 100644
> --- a/minibar.el
> +++ b/minibar.el
> @@ -137,9 +137,7 @@ a string to display, or nil in case there is to show."
>  ;;;###autoload
>  (define-minor-mode minibar-mode
>    "Toggle Minibar display."
> -  :init-value nil
>    :lighter " Minibar"
> -  :keymap nil
>    :global t
>    (if minibar-mode
>        (progn

Done.

> @@ -450,6 +448,9 @@ when it was recorded.")
>                 (lambda (i)
>                   (let ((load (minibar--module-cpu-calculate-load
>                                (format "cpu%i" i))))
> +		   ;; You should be able to simplify this with an
> +		   ;; `alist-get' call and something like #'> as the
> +		   ;; equality test.
>                     (cond
>                      ((and (char-displayable-p ?█)  ; #x2588
>                            (>= load 87.5))

Done with seq-some.

>
>
> The rest looks fine, so I'll push the commits to nongnu.git.
>

Thank you very much for your hard work reviewing all the packages.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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
  0 siblings, 2 replies; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-27  8:11 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

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



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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 18:40                   ` [NonGNU ELPA] 12 " Akib Azmain Turja
  1 sibling, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-27 11:45 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
> [...]
>
>> I just realised that none of the patches can be applies, because the
>> last commit in nongnu.git changed the package names from strings to
>> symbols.  This makes applying the patches a nuisance.  Could you perhaps
>> update the patches to use symbol for names, and make sure nothing has
>> changed?
>>
>
> Hmm, I think it's a good decision to use symbols.
>
> It's already midnight here, so I'll update the patches tomorrow.  

Sure, no problem.  Thank you for updating the patches at all.

>                                                                   BTW,
> can you please you review my Eat package[1] (terminal emulator, you have
> probably noticed it)?  I'm going to submit that too with the next batch
> of patches.

Looks interesting!  Here are my comments:


[-- Attachment #2: comments.diff --]
[-- Type: text/x-diff, Size: 24000 bytes --]

diff --git a/eat.el b/eat.el
index aa048a4..99b681e 100644
--- a/eat.el
+++ b/eat.el
@@ -114,7 +114,7 @@ This is the default name used when running Eat."
   :type 'boolean
   :group 'eat-ui)
 
-(defcustom eat-term-scrollback-size 131072 ; 128 K
+(defcustom eat-term-scrollback-size (* 128 1024) ; 128 K
   "Size of scrollback area in characters.  nil means unlimited."
   :type '(choice integer (const nil))
   :group 'eat-term
@@ -161,6 +161,7 @@ This is left disabled for security reasons."
   "Custom type specification for Eat's cursor type variables.")
 
 (defcustom eat-default-cursor-type
+  ;; Why are you checking the `default-value'?
   `(,(default-value 'cursor-type) nil nil)
   "Cursor to use in Eat buffer.
 
@@ -198,6 +199,8 @@ blinking frequency of cursor."
   :group 'eat-ui
   :group 'eat-ehell)
 
+;; Do you think combining this and `eat-maximum-latency' into a single
+;; variable makes sense?
 (defcustom eat-minimum-latency 0.008
   "Minimum display latency in seconds.
 
@@ -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."
@@ -1732,6 +1739,9 @@ Treat LINE FEED (?\\n) as the line delimiter."
                         (< (point) (point-max)))
                     (< moved n))
           (cl-incf moved)
+          ;; I have a hunch that there should be a simpler way to do
+          ;; this.  Something involving `line-end-position' and/or
+          ;; `forward-line'.
           (and (search-forward "\n" nil 'move)
                (= moved n)
                (goto-char (match-beginning 0))))
@@ -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)
@@ -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))))
 
@@ -1841,6 +1851,7 @@ where `*' indicates point."
                 (point) 'eat--t-wrap-line nil limit)))
   ;; Remove the newline.
   (when (< (point) (or limit (point-max)))
+    ;; What is the point of using `1value' here?
     (1value (cl-assert (1value (= (1value (char-after)) ?\n))))
     (delete-char 1)))
 
@@ -1855,7 +1866,7 @@ For example: when THRESHOLD is 3, \"*foobarbaz\" is converted to
       ;; Go to the threshold column.
       (eat--t-goto-col threshold)
       ;; Are we at the end of line?
-      (if (eq (char-after) ?\n)
+      (if (eq (char-after) ?\n)         ;`eolp'?
           ;; We are already at the end of line, so move to the next
           ;; line and start from the beginning.
           (forward-char)
@@ -1981,7 +1992,7 @@ Don't `set' it, bind it to a value with `let'.")
 
 (defun eat--t-reset ()
   "Reset terminal."
-  (let* ((disp (eat--t-term-display eat--t-term)))
+  (let ((disp (eat--t-term-display eat--t-term)))
     ;; Reset most of the things to their respective default values.
     (setf (eat--t-term-parser-state eat--t-term) nil)
     (setf (eat--t-disp-begin disp) (point-min-marker))
@@ -2011,7 +2022,7 @@ Don't `set' it, bind it to a value with `let'.")
     (setf (eat--t-term-mouse-encoding eat--t-term) nil)
     (setf (eat--t-term-focus-event-mode eat--t-term) nil)
     ;; Clear everything.
-    (delete-region (point-min) (point-max))
+    (erase-buffer)
     ;; Inform the UI about our new state.
     (funcall (eat--t-term-grab-mouse-fn eat--t-term) eat--t-term nil)
     (funcall (eat--t-term-set-focus-ev-mode-fn eat--t-term)
@@ -2036,7 +2047,7 @@ display."
     (unless (zerop n)
       ;; Move to the Nth next column, use spaces to reach that column
       ;; if needed.
-      (eat--t-repeated-insert ?\  (- n (eat--t-col-motion n)))
+      (eat--t-repeated-insert ?\s (- n (eat--t-col-motion n)))
       (cl-incf (eat--t-cur-x cursor) n))))
 
 (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))
 
 (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)
                                                '((?+ . ?→)
                                                  (?, . ?←)
@@ -2476,7 +2489,7 @@ N default to 1."
               (eat--t-carriage-return)
             ;; If we are somehow moved from the end of terminal,
             ;; `eat--t-beg-of-next-line' is the best option.
-            (if (/= (point) (point-max))
+            (if (/= (point) (point-max)) ;(eobp)?
                 (eat--t-beg-of-next-line 1)
               ;; We are still at the end!  We can can simply insert a
               ;; newline and update the cursor position.
@@ -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))
@@ -2631,14 +2644,14 @@ to (1, 1).  When N is 3, also erase the scrollback."
          ;; integer.
          (let ((pos (point)))
            ;; Fill current line.
-           (eat--t-repeated-insert ?\  (1+ (- (eat--t-disp-width disp)
+           (eat--t-repeated-insert ?\s (1+ (- (eat--t-disp-width disp)
                                               (eat--t-cur-x cursor)))
                                    (eat--t-face-face face))
            ;; Fill the following lines.
            (dotimes (_ (- (eat--t-disp-height disp)
                           (eat--t-cur-y cursor)))
              (insert ?\n)
-             (eat--t-repeated-insert ?\  (eat--t-disp-width disp)
+             (eat--t-repeated-insert ?\s (eat--t-disp-width disp)
                                      (eat--t-face-face face)))
            ;; Restore position.
            (goto-char pos))))
@@ -2656,7 +2669,7 @@ to (1, 1).  When N is 3, also erase the scrollback."
          (if (not (eat--t-face-bg face))
              (eat--t-repeated-insert ?\n (1- y))
            (dotimes (_ (1- y))
-             (eat--t-repeated-insert ?\  (eat--t-disp-width disp)
+             (eat--t-repeated-insert ?\s (eat--t-disp-width disp)
                                      (eat--t-face-face face))
              (insert ?\n)))
          ;; Fill the current line to keep the cursor unmoved.  Use
@@ -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)
@@ -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))))))
 
@@ -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)
@@ -3327,6 +3339,8 @@ output."
   (funcall
    (eat--t-term-input-fn eat--t-term) eat--t-term
    (let ((rgb (color-values (face-foreground 'default))))
+     ;; I wonder if it would make sense to write a `rx'-like macro for
+     ;; generating these escape codes.
      (format "\e]10;%04x/%04x/%04x\e\\"
              (pop rgb) (pop rgb) (pop rgb)))))
 
@@ -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) ?\\)
@@ -3937,7 +3949,7 @@ DATA is the selection data encoded in base64."
         ;; Calculate the beginning position of display.
         (goto-char (point-max))
         ;; TODO: This part needs explanation.
-        (let* ((disp-begin (car (eat--t-bol (- (1- height))))))
+        (let ((disp-begin (car (eat--t-bol (- (1- height))))))
           (when (< (eat--t-disp-begin disp) disp-begin)
             (goto-char (max (- (eat--t-disp-begin disp) 1)
                             (point-min)))
@@ -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
@@ -4038,6 +4054,7 @@ To set it, use (`setf' (`eat-term-input-function' TERMINAL) FUNCTION),
 where FUNCTION is the input function."
   (eat--t-term-input-fn terminal))
 
+;; Perhaps require `gv' at the top?
 (gv-define-setter eat-term-input-function (function terminal)
   `(setf (eat--t-term-input-fn ,terminal) ,function))
 
@@ -4386,8 +4403,8 @@ client process may get confused."
                     (setq tmp (get char 'ascii-character))
                     (setq char tmp))))
            (when (numberp char)
-             (let* ((base (event-basic-type char))
-                    (mods (event-modifiers char)))
+             (let ((base (event-basic-type char))
+                   (mods (event-modifiers char)))
                ;; Try to avoid event-convert-list if possible.
                (if (and (characterp char)
                         (not (memq 'meta mods))
@@ -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))))
@@ -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.
@@ -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
@@ -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
@@ -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
@@ -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)
@@ -5140,6 +5156,7 @@ ARG is passed to `yank-pop', which see."
 ;; and commentary.
 (defvar eat-mode-map
   (let ((map (make-sparse-keymap)))
+    ;; Why not use `kbd'?
     (define-key map [?\C-c ?\M-d] #'eat-char-mode)
     (define-key map [?\C-c ?\C-j] #'eat-semi-char-mode)
     (define-key map [?\C-c ?\C-k] #'eat-kill-process)
@@ -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))
 
 (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))
 
 \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
@@ -5469,7 +5487,7 @@ OS's."
         (l (length string)))
     (while (< i l)
       (process-send-string process (substring string i (min j l)))
-      (accept-process-output)
+      (accept-process-output)           ;does this require a timeout?
       (cl-incf i eat-input-chunk-size)
       (cl-incf j eat-input-chunk-size))))
 
@@ -5910,6 +5928,7 @@ PROGRAM can be a shell command."
             (eat-term-beginning eat--terminal)
             (eat-term-end eat--terminal)
           ;; TODO: Is `string-join' OK or should we use a loop?
+          ;; ODOT: What should be the issue with `string-join'?
           (eshell-output-filter
            process (string-join (nreverse queue))))))))
 
@@ -5941,6 +5960,9 @@ PROGRAM can be a shell command."
   (declare-function eshell-sentinel "esh-proc" (proc string))
   (when (buffer-live-p (process-buffer process))
     (with-current-buffer (process-buffer process)
+      ;; As you use `cl-letf'/`cl-letf*' with `symbol-function' quite
+      ;; frequently, perhaps it would make sense to define a macro
+      ;; that makes the process less repetitive?
       (cl-letf* ((process-send-string
                   (symbol-function #'process-send-string))
                  ((symbol-function #'process-send-string)
@@ -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
@@ -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))))))
 
@@ -6495,7 +6519,7 @@ FN is the original definition of `eat--eshell-cleanup', which see."
 (define-minor-mode eat-trace-mode
   "Toggle tracing Eat terminal."
   :global t
-  :require 'eat
+  :require 'eat                         ;why the `require', if the mode isn't autoloaded
   :lighter " Eat-Trace"
   (if eat-trace-mode
       (progn

[-- Attachment #3: Type: text/plain, Size: 96 bytes --]


> Bye, going to sleep.  Good night!
>
> Footnotes:
> [1]  https://codeberg.org/akib/emacs-eat/

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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
  0 siblings, 2 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-27 17:04 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 28860 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>> [...]
>>
>>> I just realised that none of the patches can be applies, because the
>>> last commit in nongnu.git changed the package names from strings to
>>> symbols.  This makes applying the patches a nuisance.  Could you perhaps
>>> update the patches to use symbol for names, and make sure nothing has
>>> changed?
>>>
>>
>> Hmm, I think it's a good decision to use symbols.
>>
>> It's already midnight here, so I'll update the patches tomorrow.  
>
> Sure, no problem.  Thank you for updating the patches at all.
>
>>                                                                   BTW,
>> can you please you review my Eat package[1] (terminal emulator, you have
>> probably noticed it)?  I'm going to submit that too with the next batch
>> of patches.
>
> Looks interesting!  Here are my comments:
>
> diff --git a/eat.el b/eat.el
> index aa048a4..99b681e 100644
> --- a/eat.el
> +++ b/eat.el
> @@ -114,7 +114,7 @@ This is the default name used when running Eat."
>    :type 'boolean
>    :group 'eat-ui)
>  
> -(defcustom eat-term-scrollback-size 131072 ; 128 K
> +(defcustom eat-term-scrollback-size (* 128 1024) ; 128 K
>    "Size of scrollback area in characters.  nil means unlimited."
>    :type '(choice integer (const nil))
>    :group 'eat-term

Thanks.

> @@ -161,6 +161,7 @@ This is left disabled for security reasons."
>    "Custom type specification for Eat's cursor type variables.")
>  
>  (defcustom eat-default-cursor-type
> +  ;; Why are you checking the `default-value'?
>    `(,(default-value 'cursor-type) nil nil)
>    "Cursor to use in Eat buffer.
>  

Because the current buffer (which is it?) while loading might have
cursor-type locally set.

> @@ -198,6 +199,8 @@ blinking frequency of cursor."
>    :group 'eat-ui
>    :group 'eat-ehell)
>  
> +;; Do you think combining this and `eat-maximum-latency' into a single
> +;; variable makes sense?
>  (defcustom eat-minimum-latency 0.008
>    "Minimum display latency in seconds.
>  

I think it's better to keep them separate, so that I can describe better
in docstring.  And also I can't think of any good name of the combined
variable.

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

> @@ -1732,6 +1739,9 @@ Treat LINE FEED (?\\n) as the line delimiter."
>                          (< (point) (point-max)))
>                      (< moved n))
>            (cl-incf moved)
> +          ;; I have a hunch that there should be a simpler way to do
> +          ;; this.  Something involving `line-end-position' and/or
> +          ;; `forward-line'.
>            (and (search-forward "\n" nil 'move)
>                 (= moved n)
>                 (goto-char (match-beginning 0))))

line-end-position doesn't work, it causes strange bugs that I can fix
only by put a (redisplay) before the call, something that I'm not going
to do at all for the sake of performance and reducing flickering.

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

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

Thanks, that's more clear.  TODO.

> @@ -1841,6 +1851,7 @@ where `*' indicates point."
>                  (point) 'eat--t-wrap-line nil limit)))
>    ;; Remove the newline.
>    (when (< (point) (or limit (point-max)))
> +    ;; What is the point of using `1value' here?
>      (1value (cl-assert (1value (= (1value (char-after)) ?\n))))
>      (delete-char 1)))
>  

To convince testcover.

> @@ -1855,7 +1866,7 @@ For example: when THRESHOLD is 3, \"*foobarbaz\" is converted to
>        ;; Go to the threshold column.
>        (eat--t-goto-col threshold)
>        ;; Are we at the end of line?
> -      (if (eq (char-after) ?\n)
> +      (if (eq (char-after) ?\n)         ;`eolp'?
>            ;; We are already at the end of line, so move to the next
>            ;; line and start from the beginning.
>            (forward-char)

After discovering the line-end-position bug, I'm afraid.

> @@ -1981,7 +1992,7 @@ Don't `set' it, bind it to a value with `let'.")
>  
>  (defun eat--t-reset ()
>    "Reset terminal."
> -  (let* ((disp (eat--t-term-display eat--t-term)))
> +  (let ((disp (eat--t-term-display eat--t-term)))
>      ;; Reset most of the things to their respective default values.
>      (setf (eat--t-term-parser-state eat--t-term) nil)
>      (setf (eat--t-disp-begin disp) (point-min-marker))

Oh, again...

> @@ -2011,7 +2022,7 @@ Don't `set' it, bind it to a value with `let'.")
>      (setf (eat--t-term-mouse-encoding eat--t-term) nil)
>      (setf (eat--t-term-focus-event-mode eat--t-term) nil)
>      ;; Clear everything.
> -    (delete-region (point-min) (point-max))
> +    (erase-buffer)
>      ;; Inform the UI about our new state.
>      (funcall (eat--t-term-grab-mouse-fn eat--t-term) eat--t-term nil)
>      (funcall (eat--t-term-set-focus-ev-mode-fn eat--t-term)

Obviously, no.  That'll remove narrowing, and narrowing to a
fundamentally part of Eat's design.

> @@ -2036,7 +2047,7 @@ display."
>      (unless (zerop n)
>        ;; Move to the Nth next column, use spaces to reach that column
>        ;; if needed.
> -      (eat--t-repeated-insert ?\  (- n (eat--t-col-motion n)))
> +      (eat--t-repeated-insert ?\s (- n (eat--t-col-motion n)))
>        (cl-incf (eat--t-cur-x cursor) n))))
>  

Didn't I push that?  Checking... no, my network connection didn't allow
me, and I forgot it afterwards.  Pushed.

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

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

> @@ -2476,7 +2489,7 @@ N default to 1."
>                (eat--t-carriage-return)
>              ;; If we are somehow moved from the end of terminal,
>              ;; `eat--t-beg-of-next-line' is the best option.
> -            (if (/= (point) (point-max))
> +            (if (/= (point) (point-max)) ;(eobp)?
>                  (eat--t-beg-of-next-line 1)
>                ;; We are still at the end!  We can can simply insert a
>                ;; newline and update the cursor position.

Again, I'm afraid.

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

> @@ -2631,14 +2644,14 @@ to (1, 1).  When N is 3, also erase the scrollback."
>           ;; integer.
>           (let ((pos (point)))
>             ;; Fill current line.
> -           (eat--t-repeated-insert ?\  (1+ (- (eat--t-disp-width disp)
> +           (eat--t-repeated-insert ?\s (1+ (- (eat--t-disp-width disp)
>                                                (eat--t-cur-x cursor)))
>                                     (eat--t-face-face face))
>             ;; Fill the following lines.
>             (dotimes (_ (- (eat--t-disp-height disp)
>                            (eat--t-cur-y cursor)))
>               (insert ?\n)
> -             (eat--t-repeated-insert ?\  (eat--t-disp-width disp)
> +             (eat--t-repeated-insert ?\s (eat--t-disp-width disp)
>                                       (eat--t-face-face face)))
>             ;; Restore position.
>             (goto-char pos))))
> @@ -2656,7 +2669,7 @@ to (1, 1).  When N is 3, also erase the scrollback."
>           (if (not (eat--t-face-bg face))
>               (eat--t-repeated-insert ?\n (1- y))
>             (dotimes (_ (1- y))
> -             (eat--t-repeated-insert ?\  (eat--t-disp-width disp)
> +             (eat--t-repeated-insert ?\s (eat--t-disp-width disp)
>                                       (eat--t-face-face face))
>               (insert ?\n)))
>           ;; Fill the current line to keep the cursor unmoved.  Use

Pushed.

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

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

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

> @@ -3327,6 +3339,8 @@ output."
>    (funcall
>     (eat--t-term-input-fn eat--t-term) eat--t-term
>     (let ((rgb (color-values (face-foreground 'default))))
> +     ;; I wonder if it would make sense to write a `rx'-like macro for
> +     ;; generating these escape codes.
>       (format "\e]10;%04x/%04x/%04x\e\\"
>               (pop rgb) (pop rgb) (pop rgb)))))
>  

rx generates regexp that (sort of) parses string.  Do you the something
that does the opposite, i.e. joins parts into string?

But yes, there indeed a bit repetition.

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

> @@ -3937,7 +3949,7 @@ DATA is the selection data encoded in base64."
>          ;; Calculate the beginning position of display.
>          (goto-char (point-max))
>          ;; TODO: This part needs explanation.
> -        (let* ((disp-begin (car (eat--t-bol (- (1- height))))))
> +        (let ((disp-begin (car (eat--t-bol (- (1- height))))))
>            (when (< (eat--t-disp-begin disp) disp-begin)
>              (goto-char (max (- (eat--t-disp-begin disp) 1)
>                              (point-min)))

Again...

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

> @@ -4038,6 +4054,7 @@ To set it, use (`setf' (`eat-term-input-function' TERMINAL) FUNCTION),
>  where FUNCTION is the input function."
>    (eat--t-term-input-fn terminal))
>  
> +;; Perhaps require `gv' at the top?
>  (gv-define-setter eat-term-input-function (function terminal)
>    `(setf (eat--t-term-input-fn ,terminal) ,function))
>  

What do you mean by "top?"  At the top of the file?  What's the
advantage of that?

> @@ -4386,8 +4403,8 @@ client process may get confused."
>                      (setq tmp (get char 'ascii-character))
>                      (setq char tmp))))
>             (when (numberp char)
> -             (let* ((base (event-basic-type char))
> -                    (mods (event-modifiers char)))
> +             (let ((base (event-basic-type char))
> +                   (mods (event-modifiers char)))
>                 ;; Try to avoid event-convert-list if possible.
>                 (if (and (characterp char)
>                          (not (memq 'meta mods))

Again...

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

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

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

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

> @@ -5140,6 +5156,7 @@ ARG is passed to `yank-pop', which see."
>  ;; and commentary.
>  (defvar eat-mode-map
>    (let ((map (make-sparse-keymap)))
> +    ;; Why not use `kbd'?
>      (define-key map [?\C-c ?\M-d] #'eat-char-mode)
>      (define-key map [?\C-c ?\C-j] #'eat-semi-char-mode)
>      (define-key map [?\C-c ?\C-k] #'eat-kill-process)

I don't know, I somehow still prefer this format in Eat.

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

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

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

> @@ -5469,7 +5487,7 @@ OS's."
>          (l (length string)))
>      (while (< i l)
>        (process-send-string process (substring string i (min j l)))
> -      (accept-process-output)
> +      (accept-process-output)           ;does this require a timeout?
>        (cl-incf i eat-input-chunk-size)
>        (cl-incf j eat-input-chunk-size))))
>  

Shamelessly stolen from Term mode.

> @@ -5910,6 +5928,7 @@ PROGRAM can be a shell command."
>              (eat-term-beginning eat--terminal)
>              (eat-term-end eat--terminal)
>            ;; TODO: Is `string-join' OK or should we use a loop?
> +          ;; ODOT: What should be the issue with `string-join'?
>            (eshell-output-filter
>             process (string-join (nreverse queue))))))))
>  

Ha ha, ODOT.  :)
Nothing, that's a reminder for me find out the most efficient code.
string-join creates garbage by throw all the string away, and using a
loop might cause Eshell to generate more garbage.

> @@ -5941,6 +5960,9 @@ PROGRAM can be a shell command."
>    (declare-function eshell-sentinel "esh-proc" (proc string))
>    (when (buffer-live-p (process-buffer process))
>      (with-current-buffer (process-buffer process)
> +      ;; As you use `cl-letf'/`cl-letf*' with `symbol-function' quite
> +      ;; frequently, perhaps it would make sense to define a macro
> +      ;; that makes the process less repetitive?
>        (cl-letf* ((process-send-string
>                    (symbol-function #'process-send-string))
>                   ((symbol-function #'process-send-string)

I was thinking that I would be nice to have a "advice-let" package.

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

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

> @@ -6495,7 +6519,7 @@ FN is the original definition of `eat--eshell-cleanup', which see."
>  (define-minor-mode eat-trace-mode
>    "Toggle tracing Eat terminal."
>    :global t
> -  :require 'eat
> +  :require 'eat                         ;why the `require', if the mode isn't autoloaded
>    :lighter " Eat-Trace"
>    (if eat-trace-mode
>        (progn

To shut package-lint up.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 12 new packages!
  2022-11-26 21:12                 ` Akib Azmain Turja
  2022-11-27 11:45                   ` Philip Kaludercic
@ 2022-11-27 18:40                   ` Akib Azmain Turja
  2022-11-27 20:36                     ` Philip Kaludercic
  1 sibling, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-27 18:40 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List


[-- Attachment #1.1: Type: text/plain, Size: 1106 bytes --]

Akib Azmain Turja <akib@disroot.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
> [...]
>
>> I just realised that none of the patches can be applies, because the
>> last commit in nongnu.git changed the package names from strings to
>> symbols.  This makes applying the patches a nuisance.  Could you perhaps

And the symbols followed by :url on same line creates a gnuisance for
Emacs to indent the following keywords.  So the :url part should be on
it own line.

>> update the patches to use symbol for names, and make sure nothing has
>> changed?
>>
>
> Hmm, I think it's a good decision to use symbols.
>
> It's already midnight here, so I'll update the patches tomorrow.  BTW,
> can you please you review my Eat package[1] (terminal emulator, you have
> probably noticed it)?  I'm going to submit that too with the next batch
> of patches.
>
> Bye, going to sleep.  Good night!
>
> Footnotes:
> [1]  https://codeberg.org/akib/emacs-eat/

Here you go, twelve patches for twelve new packages!
BTW, thanks for taking time for reviewing my packages and teaching me
some cool new things!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Blow --]
[-- Type: text/x-patch, Size: 692 bytes --]

From 56168a47dc53d41e7e75d813d68a3a13787e1ac6 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:23:12 +0600
Subject: [PATCH 01/12] * elpa-packages (blow): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 6a63ae9..b9dd434 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -52,6 +52,8 @@
 
  (bison-mode		:url "https://github.com/Wilfred/bison-mode")
 
+ (blow                  :url "https://codeberg.org/akib/emacs-blow")
+
  (boxquote		:url "https://github.com/davep/boxquote.el.git"
   :readme "README.md"
   :ignored-files ("COPYING"))
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: Camera --]
[-- Type: text/x-patch, Size: 767 bytes --]

From 19d3bf514846ed13bd70a0ed7fc28af22ac313cb Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:23:44 +0600
Subject: [PATCH 02/12] * elpa-packages (camera): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index b9dd434..314b277 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -61,6 +61,8 @@
  (buttercup		:url "https://github.com/jorgenschaefer/emacs-buttercup"
   :ignored-files ("LICENSE"))
 
+ (camera                :url "https://codeberg.org/akib/emacs-camera")
+
  (caml			:url "https://github.com/ocaml/caml-mode"
   :ignored-files ("COPYING")
   ;; The version 4.7.1 from Melpa-stable seems to correspond to
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: Devhelp --]
[-- Type: text/x-patch, Size: 704 bytes --]

From 1f688bbd722642e887b45a00407f2a89ce7bb5a0 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:24:00 +0600
Subject: [PATCH 03/12] * elpa-packages (devhelp): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 314b277..61baf51 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -101,6 +101,8 @@
   :ignored-files ("LICENSE" "test" "Cask" "Makefile")
   :news "CHANGELOG.md")
 
+ (devhelp               :url "https://codeberg.org/akib/emacs-devhelp")
+
  (diff-ansi		:url "https://codeberg.org/ideasman42/emacs-diff-ansi"
   :ignored-files ("LICENSE"))
 
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.5: GC Buffers --]
[-- Type: text/x-patch, Size: 693 bytes --]

From 950d42373a14f5bddb7274c18019615b987a05c7 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:24:15 +0600
Subject: [PATCH 04/12] * elpa-packages (gc-buffers): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 61baf51..3629275 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -190,6 +190,8 @@
 
  (free-keys		:url "https://github.com/Fuco1/free-keys")
 
+ (gc-buffers            :url "https://codeberg.org/akib/emacs-gc-buffers")
+
  (geiser		:url "https://gitlab.com/emacs-geiser/geiser.git"
   :lisp-dir "elisp"
   :readme "readme.org"
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.6: GNU Indent --]
[-- Type: text/x-patch, Size: 769 bytes --]

From 08b3171e64e28e641f2f219da4d4cbb611e11bd8 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:24:55 +0600
Subject: [PATCH 05/12] * elpa-packages (gnu-indent): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 3629275..e0bd000 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -225,6 +225,8 @@
   ;; :doc "texi/gnu-apl-mode.texi" ; the manual is currently empty
   :ignored-files ("Makefile"))
 
+ (gnu-indent  	        :url "https://codeberg.org/akib/emacs-gnu-indent")
+
  (gnuplot		:url "https://github.com/emacs-gnuplot/gnuplot"
   :ignored-files ("LICENSE" "Makefile" "gpelcard.tex")
   :news "CHANGELOG.org")
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.7: Hl Column --]
[-- Type: text/x-patch, Size: 720 bytes --]

From df4c348f3bb92d6012d09e1e80a0d0c5fb00dc7b Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:25:08 +0600
Subject: [PATCH 06/12] * elpa-packages (hl-column): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index e0bd000..08b45f8 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -294,6 +294,8 @@
   :readme "README.md"
   :ignored-files ("LICENSE"))
 
+ (hl-column             :url "https://codeberg.org/akib/emacs-hl-column")
+
  (hl-block-mode		:url "https://codeberg.org/ideasman42/emacs-hl-block-mode")
 
  (htmlize		:url "https://github.com/hniksic/emacs-htmlize"
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.8: IWindow --]
[-- Type: text/x-patch, Size: 684 bytes --]

From 7a7a22cc821f2696457d8fc844bec945bb65b996 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:25:18 +0600
Subject: [PATCH 07/12] * elpa-packages (iwindow): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 08b45f8..1dfdbf7 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -323,6 +323,8 @@
 
  (inkpot-theme		:url "https://codeberg.org/ideasman42/emacs-theme-inkpot")
 
+ (iwindow               :url "https://codeberg.org/akib/emacs-iwindow")
+
  (j-mode		:url "https://github.com/zellio/j-mode"
   :ignored-files ("LICENSE"))
 
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.9: Minibar --]
[-- Type: text/x-patch, Size: 686 bytes --]

From 8f1a93fd842fc23e6d364655a9cb957aea6566a7 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:25:31 +0600
Subject: [PATCH 08/12] * elpa-packages (minibar): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 1dfdbf7..253b247 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -400,6 +400,8 @@
 
  (mentor		:url "https://github.com/skangas/mentor.git")
 
+ (minibar               :url "https://codeberg.org/akib/emacs-minibar")
+
  (moe-theme		:url "https://github.com/kuanyui/moe-theme.el.git"
   :ignored-files ("pics" "LICENSE"))
 
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.10: testcover-mark-line --]
[-- Type: text/x-patch, Size: 737 bytes --]

From d9f8b0b2c86ba3b1e1d1935b30a8637410d7fafe Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:25:48 +0600
Subject: [PATCH 09/12] * elpa-packages (testcover-mark-line): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 253b247..f85fcfc 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -632,6 +632,8 @@
   :readme "readme.org"
   :ignored-files ("COPYING" "screenshots"))
 
+ (testcover-mark-line   :url "https://codeberg.org/akib/emacs-testcover-mark-line")
+
  (textile-mode		:url "https://github.com/juba/textile-mode")
 
  (toc-org		:url "https://github.com/snosov1/toc-org.git"
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.11: Why This --]
[-- Type: text/x-patch, Size: 776 bytes --]

From 84720c5d3cbedd58f0d1ec9b54026b38801028d9 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:26:22 +0600
Subject: [PATCH 10/12] * elpa-packages (why-this): New package

---
 elpa-packages | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index f85fcfc..92b6e13 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -681,6 +681,8 @@
  (wgrep			:url "https://github.com/mhayashi1120/Emacs-wgrep"
   :ignored-files "COPYING")
 
+ (why-this              :url "https://codeberg.org/akib/emacs-why-this")
+
  (with-editor		:url "https://github.com/magit/with-editor"
   :ignored-files ("LICENSE" "htmlxref.cnf" ".travis.yml" ".mailmap" "Makefile")
   :lisp-dir "lisp"
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.12: Workroom --]
[-- Type: text/x-patch, Size: 786 bytes --]

From 7b36db2e5e4746b27bc3df4c6003f12778b18f89 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:26:29 +0600
Subject: [PATCH 11/12] * elpa-packages (workroom): New package

---
 elpa-packages | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index 92b6e13..aa81005 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -690,6 +690,9 @@
 
  (with-simulated-input	:url "https://github.com/DarwinAwardWinner/with-simulated-input")
 
+ (workroom              :url "https://codeberg.org/akib/emacs-workroom"
+  :doc "workroom.texi")
+
  (ws-butler		:url "https://github.com/lewang/ws-butler"
   :readme "README.md"
   :ignored-files ("COPYING" "tests" "Makefile" ".travis.yml"))
-- 
2.37.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.13: Eat --]
[-- Type: text/x-patch, Size: 797 bytes --]

From 11988e41f678783d250211847363adc70025fb70 Mon Sep 17 00:00:00 2001
From: Akib Azmain Turja <akib@disroot.org>
Date: Mon, 28 Nov 2022 00:27:55 +0600
Subject: [PATCH 12/12] * elpa-packages (eat): New package

---
 elpa-packages | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index aa81005..36d5f25 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -117,6 +117,9 @@
  (drupal-mode		:url "https://github.com/arnested/drupal-mode.git"
   :ignored-files (".travis.yml" "COPYING" "Cask"))
 
+ (eat                   :url "https://codeberg.org/akib/emacs-eat"
+  :doc "eat.texi")
+
  (edit-indirect		:url "https://github.com/Fanael/edit-indirect")
 
  (editorconfig		:url "https://github.com/editorconfig/editorconfig-emacs"
-- 
2.37.1


[-- Attachment #1.14: Type: text/plain, Size: 196 bytes --]


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

^ permalink raw reply related	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-27  8:11                 ` Philip Kaludercic
@ 2022-11-27 19:22                   ` Akib Azmain Turja
  2022-11-27 19:55                   ` Akib Azmain Turja
  1 sibling, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-27 19:22 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 3012 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

[...]

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

Pushed, with you attributed as the author.

[...]

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

Done.  Does anybody know the Nix directories to add?

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

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  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
  1 sibling, 1 reply; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-27 19:55 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 7313 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-27 17:04                     ` Akib Azmain Turja
@ 2022-11-27 20:26                       ` Philip Kaludercic
  2022-11-28 19:07                       ` Akib Azmain Turja
  1 sibling, 0 replies; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-27 20:26 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

>> @@ -161,6 +161,7 @@ This is left disabled for security reasons."
>>    "Custom type specification for Eat's cursor type variables.")
>>  
>>  (defcustom eat-default-cursor-type
>> +  ;; Why are you checking the `default-value'?
>>    `(,(default-value 'cursor-type) nil nil)
>>    "Cursor to use in Eat buffer.
>>  
>
> Because the current buffer (which is it?) while loading might have
> cursor-type locally set.

That make sense.

>> @@ -198,6 +199,8 @@ blinking frequency of cursor."
>>    :group 'eat-ui
>>    :group 'eat-ehell)
>>  
>> +;; Do you think combining this and `eat-maximum-latency' into a single
>> +;; variable makes sense?
>>  (defcustom eat-minimum-latency 0.008
>>    "Minimum display latency in seconds.
>>  
>
> I think it's better to keep them separate, so that I can describe better
> in docstring.  And also I can't think of any good name of the combined
> variable.

If you think they should be separate, so be they.  But a intuitive
name might have been `eat-minimum'.

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

FYI the changes I proposed aren't exhaustive.  It might be that I
proposed a change once that could be made multiple times.

>> @@ -1841,6 +1851,7 @@ where `*' indicates point."
>>                  (point) 'eat--t-wrap-line nil limit)))
>>    ;; Remove the newline.
>>    (when (< (point) (or limit (point-max)))
>> +    ;; What is the point of using `1value' here?
>>      (1value (cl-assert (1value (= (1value (char-after)) ?\n))))
>>      (delete-char 1)))
>>  
>
> To convince testcover.

Ah, OK.  I have never used testcover (or honestly heard of it), so I was confused.

>> @@ -2011,7 +2022,7 @@ Don't `set' it, bind it to a value with `let'.")
>>      (setf (eat--t-term-mouse-encoding eat--t-term) nil)
>>      (setf (eat--t-term-focus-event-mode eat--t-term) nil)
>>      ;; Clear everything.
>> -    (delete-region (point-min) (point-max))
>> +    (erase-buffer)
>>      ;; Inform the UI about our new state.
>>      (funcall (eat--t-term-grab-mouse-fn eat--t-term) eat--t-term nil)
>>      (funcall (eat--t-term-set-focus-ev-mode-fn eat--t-term)
>
> Obviously, no.  That'll remove narrowing, and narrowing to a
> fundamentally part of Eat's design.

I see, I missed that detail (all my comments are the result of my static
analysis).

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

Again, this might also apply to a few other place, but you'd know that best.

>>  (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.
>
>> @@ -2476,7 +2489,7 @@ N default to 1."
>>                (eat--t-carriage-return)
>>              ;; If we are somehow moved from the end of terminal,
>>              ;; `eat--t-beg-of-next-line' is the best option.
>> -            (if (/= (point) (point-max))
>> +            (if (/= (point) (point-max)) ;(eobp)?
>>                  (eat--t-beg-of-next-line 1)
>>                ;; We are still at the end!  We can can simply insert a
>>                ;; newline and update the cursor position.
>
> Again, I'm afraid.

Even though `eobp' respects narrowed buffers?

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

Yes.

>> @@ -3327,6 +3339,8 @@ output."
>>    (funcall
>>     (eat--t-term-input-fn eat--t-term) eat--t-term
>>     (let ((rgb (color-values (face-foreground 'default))))
>> +     ;; I wonder if it would make sense to write a `rx'-like macro for
>> +     ;; generating these escape codes.
>>       (format "\e]10;%04x/%04x/%04x\e\\"
>>               (pop rgb) (pop rgb) (pop rgb)))))
>>  
>
> rx generates regexp that (sort of) parses string.  Do you the something
> that does the opposite, i.e. joins parts into string?
>
> But yes, there indeed a bit repetition.

A macro or pure function that is in "the spirit of rx", converts a
symbolic expression into a textual encoding.  What I was thinking of
here was something like

  (eat-cc 'color 255 0 100) "^[]10;00ff/0000/0064^[\\"

You obviously know more about control codes that I do, so you'll be a
better judge of how this should look like.

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

I guess because a multi-form setq requires a single funcall?

>> @@ -3937,7 +3949,7 @@ DATA is the selection data encoded in base64."
>>          ;; Calculate the beginning position of display.
>>          (goto-char (point-max))
>>          ;; TODO: This part needs explanation.
>> -        (let* ((disp-begin (car (eat--t-bol (- (1- height))))))
>> +        (let ((disp-begin (car (eat--t-bol (- (1- height))))))
>>            (when (< (eat--t-disp-begin disp) disp-begin)
>>              (goto-char (max (- (eat--t-disp-begin disp) 1)
>>                              (point-min)))
>
> Again...

I hope my comments don't sound too "harsh".  Even if I just changed
something without an explanation, I hope you won't interpret it as me
being annoyed.  I am sure if you look around in my code there are many
instances of me making these kinds of innocent mistakes.  It is just
easier for a fresh part of eyes to detect these, than someone who has a
loaded a mental image of the code's logic in their head.

>> @@ -4038,6 +4054,7 @@ To set it, use (`setf' (`eat-term-input-function' TERMINAL) FUNCTION),
>>  where FUNCTION is the input function."
>>    (eat--t-term-input-fn terminal))
>>  
>> +;; Perhaps require `gv' at the top?
>>  (gv-define-setter eat-term-input-function (function terminal)
>>    `(setf (eat--t-term-input-fn ,terminal) ,function))
>>  
>
> What do you mean by "top?"  At the top of the file?  What's the
> advantage of that?

Top of the file.  I just find it cleaner, because then you'll avoid
autoloading a definition midway through loading a file.  It might also
be possible, that in this case the `require' call could be wrapped in a
`eval-when-compile', but I am not sure about that.

>> @@ -5140,6 +5156,7 @@ ARG is passed to `yank-pop', which see."
>>  ;; and commentary.
>>  (defvar eat-mode-map
>>    (let ((map (make-sparse-keymap)))
>> +    ;; Why not use `kbd'?
>>      (define-key map [?\C-c ?\M-d] #'eat-char-mode)
>>      (define-key map [?\C-c ?\C-j] #'eat-semi-char-mode)
>>      (define-key map [?\C-c ?\C-k] #'eat-kill-process)
>
> I don't know, I somehow still prefer this format in Eat.

That is fine, but you used `kbd' somewhere later on, so I wasn't sure
which you preferred.

>> @@ -5910,6 +5928,7 @@ PROGRAM can be a shell command."
>>              (eat-term-beginning eat--terminal)
>>              (eat-term-end eat--terminal)
>>            ;; TODO: Is `string-join' OK or should we use a loop?
>> +          ;; ODOT: What should be the issue with `string-join'?
>>            (eshell-output-filter
>>             process (string-join (nreverse queue))))))))
>>  
>
> Ha ha, ODOT.  :)
> Nothing, that's a reminder for me find out the most efficient code.
> string-join creates garbage by throw all the string away, and using a
> loop might cause Eshell to generate more garbage.

I didn't consider it from that perspective.  If performance is of that
importance, then think about it, but my hunch is that I/O will
overshadow the overhead created by garbage, be it from strings or from a
temporary buffer.

>> @@ -6495,7 +6519,7 @@ FN is the original definition of `eat--eshell-cleanup', which see."
>>  (define-minor-mode eat-trace-mode
>>    "Toggle tracing Eat terminal."
>>    :global t
>> -  :require 'eat
>> +  :require 'eat                         ;why the `require', if the mode isn't autoloaded
>>    :lighter " Eat-Trace"
>>    (if eat-trace-mode
>>        (progn
>
> To shut package-lint up.

I am surprised to hear that.  Is this perhaps a bug in package-lint, or
did it have a good reason to recommend this?



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-27 19:55                   ` Akib Azmain Turja
@ 2022-11-27 20:30                     ` Philip Kaludercic
  0 siblings, 0 replies; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-27 20:30 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

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

What confuses me is that you manually re-create the behaviour of
blocking, which in the end probably won't even matter because indent
might even be done before you reach the first `process-live-p'.

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

Humbled, but that might actually be a good argument against this change,
if it is not that intuitive.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 12 new packages!
  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
  0 siblings, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-27 20:36 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List, Stefan Monnier

Akib Azmain Turja <akib@disroot.org> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>> [...]
>>
>>> I just realised that none of the patches can be applies, because the
>>> last commit in nongnu.git changed the package names from strings to
>>> symbols.  This makes applying the patches a nuisance.  Could you perhaps
>
> And the symbols followed by :url on same line creates a gnuisance for
> Emacs to indent the following keywords.  So the :url part should be on
> it own line.

Perhaps Stefan can comment on this?

>>> update the patches to use symbol for names, and make sure nothing has
>>> changed?
>>>
>>
>> Hmm, I think it's a good decision to use symbols.
>>
>> It's already midnight here, so I'll update the patches tomorrow.  BTW,
>> can you please you review my Eat package[1] (terminal emulator, you have
>> probably noticed it)?  I'm going to submit that too with the next batch
>> of patches.
>>
>> Bye, going to sleep.  Good night!
>>
>> Footnotes:
>> [1]  https://codeberg.org/akib/emacs-eat/
>
> Here you go, twelve patches for twelve new packages!

Thanks, and pushed.

> BTW, thanks for taking time for reviewing my packages and teaching me
> some cool new things!

Gladly, I actually learned a few things as well.  I don't remember ever
encountering macros in the wold such as `while-no-input'.



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 12 new packages!
  2022-11-27 20:36                     ` Philip Kaludercic
@ 2022-11-27 21:11                       ` Akib Azmain Turja
  0 siblings, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-27 21:11 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List, Stefan Monnier

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>> Akib Azmain Turja <akib@disroot.org> writes:
>>
>>> Philip Kaludercic <philipk@posteo.net> writes:
>>>
>>> [...]
>>>
>>>> I just realised that none of the patches can be applies, because the
>>>> last commit in nongnu.git changed the package names from strings to
>>>> symbols.  This makes applying the patches a nuisance.  Could you perhaps
>>
>> And the symbols followed by :url on same line creates a gnuisance for
>> Emacs to indent the following keywords.  So the :url part should be on
>> it own line.
>
> Perhaps Stefan can comment on this?
>
>>>> update the patches to use symbol for names, and make sure nothing has
>>>> changed?
>>>>
>>>
>>> Hmm, I think it's a good decision to use symbols.
>>>
>>> It's already midnight here, so I'll update the patches tomorrow.  BTW,
>>> can you please you review my Eat package[1] (terminal emulator, you have
>>> probably noticed it)?  I'm going to submit that too with the next batch
>>> of patches.
>>>
>>> Bye, going to sleep.  Good night!
>>>
>>> Footnotes:
>>> [1]  https://codeberg.org/akib/emacs-eat/
>>
>> Here you go, twelve patches for twelve new packages!
>
> Thanks, and pushed.
>
>> BTW, thanks for taking time for reviewing my packages and teaching me
>> some cool new things!
>
> Gladly, I actually learned a few things as well.  I don't remember ever
> encountering macros in the wold such as `while-no-input'.
>

Thank you so much!

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-27 17:04                     ` Akib Azmain Turja
  2022-11-27 20:26                       ` Philip Kaludercic
@ 2022-11-28 19:07                       ` Akib Azmain Turja
  2022-11-28 19:42                         ` Philip Kaludercic
  2022-11-28 20:57                         ` Stefan Monnier
  1 sibling, 2 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-28 19:07 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- 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 --]

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-28 19:07                       ` Akib Azmain Turja
@ 2022-11-28 19:42                         ` Philip Kaludercic
  2022-11-28 20:32                           ` Akib Azmain Turja
  2022-11-28 20:57                         ` Stefan Monnier
  1 sibling, 1 reply; 57+ messages in thread
From: Philip Kaludercic @ 2022-11-28 19:42 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Emacs Developer List

Akib Azmain Turja <akib@disroot.org> writes:

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

Any reason why not?



^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-28 19:42                         ` Philip Kaludercic
@ 2022-11-28 20:32                           ` Akib Azmain Turja
  0 siblings, 0 replies; 57+ messages in thread
From: Akib Azmain Turja @ 2022-11-28 20:32 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: Emacs Developer List

[-- Attachment #1: Type: text/plain, Size: 2229 bytes --]

Philip Kaludercic <philipk@posteo.net> writes:

> Akib Azmain Turja <akib@disroot.org> writes:
>
>>>> @@ -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.
>
> Any reason why not?
>

Not any solid reason.  I prefer to use 'setf' only with generalized
variables, and 'setf' expands to 'setq' anyway for symbols.

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

^ permalink raw reply	[flat|nested] 57+ messages in thread

* Re: [NonGNU ELPA] 11 new packages!
  2022-11-28 19:07                       ` Akib Azmain Turja
  2022-11-28 19:42                         ` Philip Kaludercic
@ 2022-11-28 20:57                         ` Stefan Monnier
  1 sibling, 0 replies; 57+ messages in thread
From: Stefan Monnier @ 2022-11-28 20:57 UTC (permalink / raw)
  To: Akib Azmain Turja; +Cc: Philip Kaludercic, Emacs Developer List

>> Somehow I prefer to use one setq for each variable.  Is setting multiple
>> at once faster?  Benchmarking with "benchmark"...  Yes, about 1.5 times.

No, the generated byte-code should be identical.  If you see
a difference it's probably because you tested interpreted code.


        Stefan




^ permalink raw reply	[flat|nested] 57+ messages in thread

end of thread, other threads:[~2022-11-28 20:57 UTC | newest]

Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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