Re: [emms-help] mpv player alternative with json-ipc

[Mike Kazantsev]

On Fri, 13 Apr 2018 15:23:05 -0400
Ian Dunn <address@hidden> wrote:

> 1a. You should require s and dash in the header, as they both appear to be 
> dependencies.  I can't immediately see any others.
> 1b. Neither is currently a dependency of EMMS, so either they'd have to be 
> added, or removed from your code.
> 
> 2. In "emms-mpv-ipc-line", give the option for users to introduce their own 
> event handlers.  This makes it easier to extend to support additional events.
> 
> 3. As stated in another email, you use the --idle flag, which seems to exist 
> for this very purpose.  Thanks for that.
> 
> 4. A general convention when dealing with external processes in emacs is to 
> break up command line args and the actual program.  It especially helps when 
> users want to use a specific binary.  Otherwise, I'd have to modify just the 
> first argument of a list if I wanted to modify it at all in a robust way.
> 
> 5. Adding to 4, I'd recommend making emms-mpv-proc-cmd a defcustom instead of 
> a defvar.

Completely agree.

Definitely need to make quite a few defvar's there into defcustom's,
including separate binary/args.

s/dash deps (pretty sure I use cl-return and let* too) are only
omitted because it's in the repo where I use them all over the place,
and would be better to drop them entirely for emms code to avoid new
requires there.

Will be sure to address all these, thanks a lot for pointing them out!


-- 
Mike Kazantsev // fraggod.net