[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: ampc back on elpa?
From: |
Stefan Monnier |
Subject: |
Re: ampc back on elpa? |
Date: |
Mon, 13 Jun 2016 08:45:16 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) |
> I adapted my code to the elpa ampc version. Patch follows. I'm not a skilled
> lisp developer, feel free to comment.
Looks pretty good, except for one spot that doesn't seem right. See below.
Stefan
> @@ -569,6 +573,13 @@ modified."
> (0.4 playlist ,@pl-prop)
> (1.0 playlists)))
> ,rs_b)
> + ("Search view"
> + ,(kbd "F")
> + horizontal
> + (0.4 vertical
> + (6 status)
> + (1.0 current-playlist ,@pl-prop))
> + ,search-view)
> ("Outputs view"
> ,(kbd "L")
> outputs :properties (("outputname" :title "Name" :min 10 :max 30)
Indentation looks odd here. Maybe a mix of spaces and tabs?
> (defmacro ampc-iterate-source-output (delimiter bindings pad-data &rest body)
> + "delimiter = what delimit command results in mpd response"
Thank you for helping document the code. Could you add a first line
describing of the general functionality? Also put `delimiter` in
all-caps since that's the convention used in Emacs for function/macro
arguments in docstrings.
> @@ -1507,6 +1535,14 @@ modified."
> (ampc-send-command 'currentsong))
> (playlists
> (ampc-send-command 'listplaylists))
> + (search
> + (if (active-minibuffer-window) ;can't find a better
> way to check minibuffer
> + (when ampc-search-keywords
> + (ampc-send-command 'search nil "any"
> (ampc-quote ampc-search-keywords)))
> + (let ((search (read-from-minibuffer "Keywords:
> ")))
> + (unless (string= "" search)
> + (setq ampc-search-keywords search)
> + (ampc-send-command 'search nil "any"
> (ampc-quote search))))))
> (current-playlist
> (ampc-send-command 'playlistinfo))))))
> (ampc-send-command 'status)
Can you rewrite it to something like
(search
(unless (active-minibuffer-window)
;; Can't find a better way to check minibuffer
(let ((search (read-from-minibuffer "Keywords: ")))
(setq ampc-search-keywords
(unless (string= "" search) search))))
(when ampc-search-keywords
(ampc-send-command 'search nil "any"
(ampc-quote ampc-search-keywords))))
But in any case, this looks fishy. `ampc-update` doesn't seem like
a good place to have user interaction. Why do you need to
`read-from-minibuffer` *here* (BTW, I recommend you use `read-string`
instead)? I mean, why can't you set ampc-search-keywords elsewhere?
Stefan