From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: ampc back on elpa? Date: Mon, 13 Jun 2016 08:45:16 -0400 Message-ID: References: <573736bd.442cc20a.8d117.ffff9cd7@mx.google.com> <5745411e.aaf0c20a.8e140.4623@mx.google.com> <575abfdd.697ac20a.d9403.5ca0@mx.google.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1465821899 28298 80.91.229.3 (13 Jun 2016 12:44:59 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 13 Jun 2016 12:44:59 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Jun 13 14:44:50 2016 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bCREY-0005nn-31 for ged-emacs-devel@m.gmane.org; Mon, 13 Jun 2016 14:44:50 +0200 Original-Received: from localhost ([::1]:56261 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCREX-00030i-Ae for ged-emacs-devel@m.gmane.org; Mon, 13 Jun 2016 08:44:49 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:41851) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCRDk-0002zR-SL for emacs-devel@gnu.org; Mon, 13 Jun 2016 08:44:01 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCRDg-0002jw-Qe for emacs-devel@gnu.org; Mon, 13 Jun 2016 08:44:00 -0400 Original-Received: from plane.gmane.org ([80.91.229.3]:55945) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCRDg-0002jp-Ja for emacs-devel@gnu.org; Mon, 13 Jun 2016 08:43:56 -0400 Original-Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1bCRDd-00057c-9e for emacs-devel@gnu.org; Mon, 13 Jun 2016 14:43:53 +0200 Original-Received: from 45.72.244.67 ([45.72.244.67]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 13 Jun 2016 14:43:53 +0200 Original-Received: from monnier by 45.72.244.67 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 13 Jun 2016 14:43:53 +0200 X-Injected-Via-Gmane: http://gmane.org/ Original-Lines: 69 Original-X-Complaints-To: usenet@ger.gmane.org X-Gmane-NNTP-Posting-Host: 45.72.244.67 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) Cancel-Lock: sha1:or+kq0y9PGaY6dmvjBqlCEOQUOc= X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 80.91.229.3 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:204334 Archived-At: > 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