[ELPA] new package: sndio.el

[ELPA] new package: sndio.el

[Omar Polo]

Hello,

Some time ago to scratch an itch I wrote sndio.el, a package to interact
with the OpenBSD' audio daemon sndiod(8).  Recently, I've enhanced it a
bit and added an hydra-like window for a quick interaction and thought
it may be useful to others, so here's the submission.  It should work
outside of OpenBSD too provided that you're running sndio and have
sndioctl(1), but I never tried.

There's a small gif on the repo that shows sndio.el in action:

	https://git.omarpolo.com/sndio.el/about/

I was able to fetch and checkout the source in the elpa repository using
the attached diff.

Thanks,

Omar Polo

diff --git a/elpa-packages b/elpa-packages
index efa53e3e40..2e21c9bffb 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -438,6 +438,7 @@
  ("smalltalk-mode"	:url nil) ;; Was "git://git.sv.gnu.org/smalltalk"
  ("smart-yank"		:url nil)
  ("sml-mode"		:url nil)
+ ("sndio"		:url "https://git.omarpolo.com/sndio.el")
  ("so-long"             :core "lisp/so-long.el")
  ("soap-client"		:core ("lisp/net/soap-client.el" "lisp/net/soap-inspect.el"))
  ("sokoban"		:url nil)

Re: [ELPA] new package: sndio.el

[Philip Kaludercic]

Omar Polo <op@omarpolo.com> writes:

> Hello,
>
> Some time ago to scratch an itch I wrote sndio.el, a package to interact
> with the OpenBSD' audio daemon sndiod(8).  Recently, I've enhanced it a
> bit and added an hydra-like window for a quick interaction and thought
> it may be useful to others, so here's the submission.  It should work
> outside of OpenBSD too provided that you're running sndio and have
> sndioctl(1), but I never tried.

From what I see, if I were to download and run the package on a GNU
System without sndio, there wouldn't be any clear error message
indicating what went wrong, just an error message indicating that
sndioctl has failed, right? (this is just from reading and using faulty
mental evaluation). If the package is to be added to GNU ELPA, you
should probably add a explicit error message (perhaps even at compile
time) to clarify what is missing, and that the package isn't mean to
work on this system.

> There's a small gif on the repo that shows sndio.el in action:
>
> 	https://git.omarpolo.com/sndio.el/about/
>
> I was able to fetch and checkout the source in the elpa repository using
> the attached diff.

The source looks good, I can add it to GNU ELPA if there are no objections.

> Thanks,
>
> Omar Polo
>
> diff --git a/elpa-packages b/elpa-packages
> index efa53e3e40..2e21c9bffb 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -438,6 +438,7 @@
>   ("smalltalk-mode"	:url nil) ;; Was "git://git.sv.gnu.org/smalltalk"
>   ("smart-yank"		:url nil)
>   ("sml-mode"		:url nil)
> + ("sndio"		:url "https://git.omarpolo.com/sndio.el")
>   ("so-long"             :core "lisp/so-long.el")
>   ("soap-client"		:core ("lisp/net/soap-client.el" "lisp/net/soap-inspect.el"))
>   ("sokoban"		:url nil)
>
>

-- 
	Philip Kaludercic

Re: [ELPA] new package: sndio.el

[Omar Polo]

Philip Kaludercic <philipk@posteo.net> writes:

> Omar Polo <op@omarpolo.com> writes:
>
>> Hello,
>>
>> Some time ago to scratch an itch I wrote sndio.el, a package to interact
>> with the OpenBSD' audio daemon sndiod(8).  Recently, I've enhanced it a
>> bit and added an hydra-like window for a quick interaction and thought
>> it may be useful to others, so here's the submission.  It should work
>> outside of OpenBSD too provided that you're running sndio and have
>> sndioctl(1), but I never tried.
>
> From what I see, if I were to download and run the package on a GNU
> System without sndio, there wouldn't be any clear error message
> indicating what went wrong, just an error message indicating that
> sndioctl has failed, right? (this is just from reading and using faulty
> mental evaluation). If the package is to be added to GNU ELPA, you
> should probably add a explicit error message (perhaps even at compile
> time) to clarify what is missing, and that the package isn't mean to
> work on this system.

Yes, as things are now if sndio-sndioctl-cmd is not found `sndio-update'
fails with a generic error due to process-file not finding the
executable.

Would something like this be a viable option?

--- sndio.el
+++ sndio.el
@@ -67,6 +67,8 @@
 (defun sndio-update ()
   "Update the current sndio buffer."
   (interactive)
+  (unless (executable-find sndio-sndioctl-cmd)
+    (error "Can't find executable %s" sndio-sndioctl-cmd))
   (when (derived-mode-p 'sndio-mode)
     (let ((inhibit-read-only t))
       (erase-buffer)

(sndio-update is the first function called in both the entrypoints of
the package, so it seems a good place for such a check)

>> There's a small gif on the repo that shows sndio.el in action:
>>
>> 	https://git.omarpolo.com/sndio.el/about/
>>
>> I was able to fetch and checkout the source in the elpa repository using
>> the attached diff.
>
> The source looks good, I can add it to GNU ELPA if there are no objections.
>
>> Thanks,
>>
>> Omar Polo
>>
>> diff --git a/elpa-packages b/elpa-packages
>> index efa53e3e40..2e21c9bffb 100644
>> --- a/elpa-packages
>> +++ b/elpa-packages
>> @@ -438,6 +438,7 @@
>>   ("smalltalk-mode"	:url nil) ;; Was "git://git.sv.gnu.org/smalltalk"
>>   ("smart-yank"		:url nil)
>>   ("sml-mode"		:url nil)
>> + ("sndio"		:url "https://git.omarpolo.com/sndio.el")
>>   ("so-long"             :core "lisp/so-long.el")
>>   ("soap-client"		:core ("lisp/net/soap-client.el" "lisp/net/soap-inspect.el"))
>>   ("sokoban"		:url nil)
>>
>>

Re: [ELPA] new package: sndio.el

[Stefan Monnier]

> From what I see, if I were to download and run the package on a GNU
> System without sndio, there wouldn't be any clear error message
> indicating what went wrong, just an error message indicating that
> sndioctl has failed, right? (this is just from reading and using faulty
> mental evaluation). If the package is to be added to GNU ELPA, you
> should probably add a explicit error message (perhaps even at compile
> time) to clarify what is missing, and that the package isn't mean to
> work on this system.

From where I stand, OTOH, I'd ask not to emit an error or warning during
compilation (e.g. I compile all the files in elpa.git and I don't have
sndio).  Similarly, don't sign an error when loading `sndio.el` (a
warning is tolerable).


        Stefan

Re: [ELPA] new package: sndio.el

[Philip Kaludercic]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> From what I see, if I were to download and run the package on a GNU
>> System without sndio, there wouldn't be any clear error message
>> indicating what went wrong, just an error message indicating that
>> sndioctl has failed, right? (this is just from reading and using faulty
>> mental evaluation). If the package is to be added to GNU ELPA, you
>> should probably add a explicit error message (perhaps even at compile
>> time) to clarify what is missing, and that the package isn't mean to
>> work on this system.
>
> From where I stand, OTOH, I'd ask not to emit an error or warning during
> compilation (e.g. I compile all the files in elpa.git and I don't have
> sndio).  Similarly, don't sign an error when loading `sndio.el` (a
> warning is tolerable).

I did not consider that.  In that case a warning/error is certainly not
acceptable.

It should probably suffice to just throw an error when calling the
autoloaded `sndio' command.

-- 
	Philip Kaludercic

Re: [ELPA] new package: sndio.el

[Stefan Monnier]

Philip Kaludercic [2021-11-14 18:32:09] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> From what I see, if I were to download and run the package on a GNU
>>> System without sndio, there wouldn't be any clear error message
>>> indicating what went wrong, just an error message indicating that
>>> sndioctl has failed, right? (this is just from reading and using faulty
>>> mental evaluation). If the package is to be added to GNU ELPA, you
>>> should probably add a explicit error message (perhaps even at compile
>>> time) to clarify what is missing, and that the package isn't mean to
>>> work on this system.
>>
>> From where I stand, OTOH, I'd ask not to emit an error or warning during
>> compilation (e.g. I compile all the files in elpa.git and I don't have
>> sndio).  Similarly, don't sign an error when loading `sndio.el` (a
>> warning is tolerable).
>
> I did not consider that.  In that case a warning/error is certainly not
> acceptable.
>
> It should probably suffice to just throw an error when calling the
> autoloaded `sndio' command.

Signaling errors when the package is actually used makes a lot of
sense, yes.

Just to be clear: I haven't looked at the code, so I'm only speaking
on matters of principles here.


        Stefan

Re: [ELPA] new package: sndio.el

[Philip Kaludercic]

Omar Polo <op@omarpolo.com> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Omar Polo <op@omarpolo.com> writes:
>>
>>> Hello,
>>>
>>> Some time ago to scratch an itch I wrote sndio.el, a package to interact
>>> with the OpenBSD' audio daemon sndiod(8).  Recently, I've enhanced it a
>>> bit and added an hydra-like window for a quick interaction and thought
>>> it may be useful to others, so here's the submission.  It should work
>>> outside of OpenBSD too provided that you're running sndio and have
>>> sndioctl(1), but I never tried.
>>
>> From what I see, if I were to download and run the package on a GNU
>> System without sndio, there wouldn't be any clear error message
>> indicating what went wrong, just an error message indicating that
>> sndioctl has failed, right? (this is just from reading and using faulty
>> mental evaluation). If the package is to be added to GNU ELPA, you
>> should probably add a explicit error message (perhaps even at compile
>> time) to clarify what is missing, and that the package isn't mean to
>> work on this system.
>
> Yes, as things are now if sndio-sndioctl-cmd is not found `sndio-update'
> fails with a generic error due to process-file not finding the
> executable.
>
> Would something like this be a viable option?
>
> --- sndio.el
> +++ sndio.el
> @@ -67,6 +67,8 @@
>  (defun sndio-update ()
>    "Update the current sndio buffer."
>    (interactive)
> +  (unless (executable-find sndio-sndioctl-cmd)
> +    (error "Can't find executable %s" sndio-sndioctl-cmd))
>    (when (derived-mode-p 'sndio-mode)
>      (let ((inhibit-read-only t))
>        (erase-buffer)
>
> (sndio-update is the first function called in both the entrypoints of
> the package, so it seems a good place for such a check)

This looks like a good place to add the check.  I just have two
questions:

1. Should an error or a user-error be raised?
2. Should the error message be more explicit, and mention that sndio
   isn't being used on the current system?

>>> There's a small gif on the repo that shows sndio.el in action:
>>>
>>> 	https://git.omarpolo.com/sndio.el/about/
>>>
>>> I was able to fetch and checkout the source in the elpa repository using
>>> the attached diff.
>>
>> The source looks good, I can add it to GNU ELPA if there are no objections.
>>
>>> Thanks,
>>>
>>> Omar Polo
>>>
>>> diff --git a/elpa-packages b/elpa-packages
>>> index efa53e3e40..2e21c9bffb 100644
>>> --- a/elpa-packages
>>> +++ b/elpa-packages
>>> @@ -438,6 +438,7 @@
>>>   ("smalltalk-mode"	:url nil) ;; Was "git://git.sv.gnu.org/smalltalk"
>>>   ("smart-yank"		:url nil)
>>>   ("sml-mode"		:url nil)
>>> + ("sndio"		:url "https://git.omarpolo.com/sndio.el")
>>>   ("so-long"             :core "lisp/so-long.el")
>>>   ("soap-client"		:core ("lisp/net/soap-client.el" "lisp/net/soap-inspect.el"))
>>>   ("sokoban"		:url nil)
>>>
>>>
>

-- 
	Philip Kaludercic

Re: [ELPA] new package: sndio.el

[Omar Polo]

Philip Kaludercic <philipk@posteo.net> writes:

> Omar Polo <op@omarpolo.com> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>>> Omar Polo <op@omarpolo.com> writes:
>>>
>>>> Hello,
>>>>
>>>> Some time ago to scratch an itch I wrote sndio.el, a package to interact
>>>> with the OpenBSD' audio daemon sndiod(8).  Recently, I've enhanced it a
>>>> bit and added an hydra-like window for a quick interaction and thought
>>>> it may be useful to others, so here's the submission.  It should work
>>>> outside of OpenBSD too provided that you're running sndio and have
>>>> sndioctl(1), but I never tried.
>>>
>>> From what I see, if I were to download and run the package on a GNU
>>> System without sndio, there wouldn't be any clear error message
>>> indicating what went wrong, just an error message indicating that
>>> sndioctl has failed, right? (this is just from reading and using faulty
>>> mental evaluation). If the package is to be added to GNU ELPA, you
>>> should probably add a explicit error message (perhaps even at compile
>>> time) to clarify what is missing, and that the package isn't mean to
>>> work on this system.
>>
>> Yes, as things are now if sndio-sndioctl-cmd is not found `sndio-update'
>> fails with a generic error due to process-file not finding the
>> executable.
>>
>> Would something like this be a viable option?
>>
>> --- sndio.el
>> +++ sndio.el
>> @@ -67,6 +67,8 @@
>>  (defun sndio-update ()
>>    "Update the current sndio buffer."
>>    (interactive)
>> +  (unless (executable-find sndio-sndioctl-cmd)
>> +    (error "Can't find executable %s" sndio-sndioctl-cmd))
>>    (when (derived-mode-p 'sndio-mode)
>>      (let ((inhibit-read-only t))
>>        (erase-buffer)
>>
>> (sndio-update is the first function called in both the entrypoints of
>> the package, so it seems a good place for such a check)
>
> This looks like a good place to add the check.  I just have two
> questions:
>
> 1. Should an error or a user-error be raised?
> 2. Should the error message be more explicit, and mention that sndio
>    isn't being used on the current system?

TIL the difference between user-error and error, I guess I have to fix
some elisp code I have around... :D

I just committed the following diff

--- sndio.el
+++ sndio.el
@@ -67,6 +67,9 @@
 (defun sndio-update ()
   "Update the current sndio buffer."
   (interactive)
+  (unless (executable-find sndio-sndioctl-cmd)
+    (user-error "Can't find executable %s, is sndio installed?"
+                sndio-sndioctl-cmd))
   (when (derived-mode-p 'sndio-mode)
     (let ((inhibit-read-only t))
       (erase-buffer)

if it's OK I can tag a version so elpa pick that :)

Thanks!