tvraman / emacspeak

Emacspeak --- The Complete Audio Desktop
245 stars 52 forks source link

Fix broken emacspeak when missing sox #111

Closed robertmeta closed 7 months ago

robertmeta commented 7 months ago

There might be a better way to fix this, but here is my attempt.

Behavior: When missing sox on MacOS, Emacspeak wouldn't entirely crash but would go into a very odd mode of working. Moving lines and lots of other things would generate no messages, and this was confirmed by using log-swiftmac to see that messages were not being sent when switching lines. Typing keys still worked and echoed.

Discovery: So, I did some bisecting and I found the problem was in the commit 11121766c. When trying to fix it, it appears to me it had two problems. The first is even when the unless code was run, it would still at least once try to execute the binary which does not exist. The second and more confusing problem is that when you run (error "some message") it triggers the breakage, and I can not understand why that creates this unique broken condition. I tried adding (interactive) to the function, it didn't change the behavior.

Solution: This patch still emits a message telling the user what is going on but everything continues to function properly and it never runs the path that attempts to use sox.

robertmeta commented 7 months ago

@tvraman I didn't actually see your replies until I had already made this (was offline for the day) but it seems I ended up at the same place anyway. Thanks for the feedback on the other PR, I meant to close it but I think I was offline by then.

tvraman commented 7 months ago

I suggest we:

  1. Document that mac users should install Sox
  2. Close this issue.

--

tvraman commented 7 months ago

Add a line to swiftmac readme asking users to install SoX

robertmeta commented 7 months ago

@tvraman the problem is, you don't need sox when using serve, yet it breaks emacspeak due to the code

    (unless emacspeak-play-program
      (setq-default emacspeak-use-auditory-icons nil)
      (error "No valid player for auditory icons."))

the error line breaks emacspeak in the way I described. Changing (error to (message makes it all work again. It is just the errror line there.

robertmeta commented 7 months ago

@tvraman additionally, the logic doesn't make sense to me, it disables icons if there isn't a local play program EVEN when using serve or queue. If using serve and the server supports playing, it doesn't need any local player. In that case not having a local player is a non-issue and you should still call the function that will do the serve or queue.

tvraman commented 7 months ago

I'm going to add the setting to use serve-icons to the mac tts config function.

There should be no references to play program for the Mac workflow after that.

Principle: dont introduce work-arounds until you understand the problem:-)

Make sure your server can handle wav and ogg files; as stated elsewhere in a few months I'll get rid of wav files for icons altogether -- but I would suggest keeping the support for wav around even then.

--

tvraman commented 7 months ago

makes sense, will get there one step at a time

--

robertmeta commented 7 months ago

TV--

OK, I am either not understanding something basic, very basic or not communicating clearly. So, here is my understanding.

So, the entry point for playing icons is:

emacspeak-auditory-icon

this function will then call the proper "real" function which is play, queue or serve if the user has audio icons turned on.

The problem is, before it does any of those things it checks for the play program, which is irrelevant in 2 of the 3 setups and worse, the error line in that path breaks emacspeak at least on mac machines.

Where am I misunderstanding?

On Feb 3, 2024, at 12:21, T. V. Raman @.***> wrote:

I'm going to add the setting to use serve-icons to the mac tts config function.

There should be no references to play program for the Mac workflow after that.

Principle: dont introduce work-arounds until you understand the problem:-)

Make sure your server can handle wav and ogg files; as stated elsewhere in a few months I'll get rid of wav files for icons altogether -- but I would suggest keeping the support for wav around even then.

-- — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

tvraman commented 7 months ago

step back, step away for a day, then look at the state of the system after today's multiple pushes. If you still find something that doesn't all hang together, we can discuss it.

--

robertmeta commented 7 months ago

Fair.

On Feb 3, 2024, at 14:31, T. V. Raman @.***> wrote:

step back, step away for a day, then look at the state of the system after today's multiple pushes. If you still find something that doesn't all hang together, we can discuss it.

-- — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>