Skip to content

Commit ae051c7

Browse files
authored
Merge pull request kstep#18 from tomprince/quoting
Be principled about formatting commands.
2 parents bab463e + b3cede4 commit ae051c7

File tree

7 files changed

+275
-153
lines changed

7 files changed

+275
-153
lines changed

src/client.rs

Lines changed: 96 additions & 93 deletions
Large diffs are not rendered by default.

src/convert.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ impl ToQueueRangeOrPlace for RangeFrom<u32> {
152152

153153
impl ToQueueRange for RangeFull {
154154
fn to_range(self) -> String {
155-
String::new()
155+
ToQueueRange::to_range(0..)
156156
}
157157
}
158158

src/idle.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,10 @@ impl FromStr for Subsystem {
8484
}
8585
}
8686

87-
impl fmt::Display for Subsystem {
88-
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
87+
impl Subsystem {
88+
fn to_str(self) -> &'static str {
8989
use self::Subsystem::*;
90-
f.write_str(match *self {
90+
match self {
9191
Database => "database",
9292
Update => "update",
9393
Playlist => "stored_playlist",
@@ -99,10 +99,24 @@ impl fmt::Display for Subsystem {
9999
Sticker => "sticker",
100100
Subscription => "subscription",
101101
Message => "message",
102-
})
102+
}
103+
}
104+
}
105+
106+
impl fmt::Display for Subsystem {
107+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
108+
f.write_str(self.to_str())
103109
}
104110
}
105111

112+
use std::result::Result as StdResult;
113+
impl<'a> ::proto::ToArguments for Subsystem {
114+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
115+
where F: FnMut(&str) -> StdResult<(), E>
116+
{
117+
f(self.to_str())
118+
}
119+
}
106120

107121
/// "Idle" mode guard enforcing MPD asynchronous events protocol
108122
pub struct IdleGuard<'a, S: 'a + Read + Write>(&'a mut Client<S>);
@@ -126,7 +140,7 @@ impl<'a, S: 'a + Read + Write> IdleGuard<'a, S> {
126140

127141
impl<'a, S: 'a + Read + Write> Drop for IdleGuard<'a, S> {
128142
fn drop(&mut self) {
129-
let _ = self.0.run_command("noidle").map(|_| self.0.drain());
143+
let _ = self.0.run_command("noidle", ()).map(|_| self.0.drain());
130144
}
131145
}
132146

@@ -164,11 +178,7 @@ pub trait Idle {
164178
impl<S: Read + Write> Idle for Client<S> {
165179
type Stream = S;
166180
fn idle<'a>(&'a mut self, subsystems: &[Subsystem]) -> Result<IdleGuard<'a, S>, Error> {
167-
let subsystems = subsystems.iter()
168-
.map(|v| v.to_string())
169-
.collect::<Vec<String>>()
170-
.join(" ");
171-
try!(self.run_command_fmt(format_args!("idle {}", subsystems)));
181+
self.run_command("idle", subsystems)?;
172182
Ok(IdleGuard(self))
173183
}
174184
}

src/proto.rs

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ use error::{Error, ProtoError, Result};
77

88
use reply::Reply;
99
use std::collections::BTreeMap;
10-
use std::fmt::Arguments;
10+
use std::fmt;
1111
use std::io::{self, Lines, Read, Write};
12+
use std::result::Result as StdResult;
1213

1314
pub struct Pairs<I>(pub I);
1415

@@ -101,8 +102,7 @@ pub trait Proto {
101102
fn read_line(&mut self) -> Result<String>;
102103
fn read_pairs(&mut self) -> Pairs<Lines<&mut BufStream<Self::Stream>>>;
103104

104-
fn run_command(&mut self, command: &str) -> Result<()>;
105-
fn run_command_fmt(&mut self, command: Arguments) -> Result<()>;
105+
fn run_command<I>(&mut self, command: &str, arguments: I) -> Result<()> where I: ToArguments;
106106

107107
fn read_map(&mut self) -> Result<BTreeMap<String, String>> {
108108
self.read_pairs().collect()
@@ -157,4 +157,93 @@ pub trait Proto {
157157
}
158158
}
159159
}
160+
161+
162+
pub trait ToArguments {
163+
fn to_arguments<F, E>(&self, &mut F) -> StdResult<(), E> where F: FnMut(&str) -> StdResult<(), E>;
164+
}
165+
166+
impl ToArguments for () {
167+
fn to_arguments<F, E>(&self, _: &mut F) -> StdResult<(), E>
168+
where F: FnMut(&str) -> StdResult<(), E>
169+
{
170+
Ok(())
171+
}
172+
}
173+
174+
impl<'a> ToArguments for &'a str {
175+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
176+
where F: FnMut(&str) -> StdResult<(), E>
177+
{
178+
f(self)
179+
}
180+
}
181+
182+
macro_rules! argument_for_display {
183+
( $x:path ) => {
184+
impl ToArguments for $x {
185+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
186+
where F: FnMut(&str) -> StdResult<(), E>
187+
{
188+
f(&self.to_string())
189+
}
190+
}
191+
};
192+
}
193+
argument_for_display!{i8}
194+
argument_for_display!{u8}
195+
argument_for_display!{u32}
196+
argument_for_display!{f32}
197+
argument_for_display!{f64}
198+
argument_for_display!{usize}
199+
argument_for_display!{::status::ReplayGain}
200+
argument_for_display!{String}
201+
argument_for_display!{::song::Id}
202+
argument_for_display!{::song::Range}
203+
argument_for_display!{::message::Channel}
204+
205+
macro_rules! argument_for_tuple {
206+
( $($t:ident: $T: ident),+ ) => {
207+
impl<$($T : ToArguments,)*> ToArguments for ($($T,)*) {
208+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
209+
where F: FnMut(&str) -> StdResult<(), E>
210+
{
211+
let ($(ref $t,)*) = *self;
212+
$(
213+
$t.to_arguments(f)?;
214+
)*
215+
Ok(())
216+
}
217+
}
218+
};
219+
}
220+
argument_for_tuple!{t0: T0}
221+
argument_for_tuple!{t0: T0, t1: T1}
222+
argument_for_tuple!{t0: T0, t1: T1, t2: T2}
223+
argument_for_tuple!{t0: T0, t1: T1, t2: T2, t3:T3}
224+
225+
impl<'a, T: ToArguments> ToArguments for &'a [T] {
226+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
227+
where F: FnMut(&str) -> StdResult<(), E>
228+
{
229+
for arg in *self {
230+
arg.to_arguments(f)?
231+
}
232+
Ok(())
233+
}
234+
}
235+
236+
pub struct Quoted<'a, D: fmt::Display + 'a + ?Sized>(pub &'a D);
237+
238+
impl<'a, D: fmt::Display + 'a + ?Sized> fmt::Display for Quoted<'a, D> {
239+
fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
240+
let unquoted = format!("{}", self.0);
241+
if &unquoted == "" {
242+
// return Ok(());
243+
}
244+
let quoted = unquoted.replace('\\', r"\\").replace('"', r#"\""#);
245+
formatter.write_fmt(format_args!("\"{}\"", &quoted))
246+
}
247+
}
248+
160249
// }}}

src/search.rs

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
#![allow(missing_docs)]
22
// TODO: unfinished functionality
33

4+
use proto::ToArguments;
45
use std::borrow::Cow;
56
use std::convert::Into;
67
use std::fmt;
8+
use std::result::Result as StdResult;
79

810
pub enum Term<'a> {
911
Any,
@@ -61,36 +63,72 @@ impl<'a> Query<'a> {
6163

6264
impl<'a> fmt::Display for Term<'a> {
6365
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
64-
match *self {
65-
Term::Any => f.write_str("any"),
66-
Term::File => f.write_str("file"),
67-
Term::Base => f.write_str("base"),
68-
Term::LastMod => f.write_str("modified-since"),
69-
Term::Tag(ref tag) => f.write_str(&*tag),
70-
}
66+
f.write_str(match *self {
67+
Term::Any => "any",
68+
Term::File => "file",
69+
Term::Base => "base",
70+
Term::LastMod => "modified-since",
71+
Term::Tag(ref tag) => &*tag,
72+
})
7173
}
7274
}
7375

74-
impl<'a> fmt::Display for Filter<'a> {
75-
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
76-
write!(f, " {} \"{}\"", self.typ, self.what)
76+
impl<'a> ToArguments for &'a Filter<'a> {
77+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
78+
where F: FnMut(&str) -> StdResult<(), E>
79+
{
80+
f(&self.typ.to_string())?;
81+
f(&self.what)
7782
}
7883
}
7984

80-
impl<'a> fmt::Display for Query<'a> {
81-
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
85+
impl<'a> ToArguments for &'a Query<'a> {
86+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
87+
where F: FnMut(&str) -> StdResult<(), E>
88+
{
8289
for filter in &self.filters {
83-
try!(filter.fmt(f));
90+
filter.to_arguments(f)?
8491
}
8592
Ok(())
8693
}
8794
}
8895

89-
impl fmt::Display for Window {
90-
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
91-
if let Some((a, b)) = self.0 {
92-
write!(f, " window {}:{}", a, b)?;
96+
impl ToArguments for Window {
97+
fn to_arguments<F, E>(&self, f: &mut F) -> StdResult<(), E>
98+
where F: FnMut(&str) -> StdResult<(), E>
99+
{
100+
if let Some(window) = self.0 {
101+
f("window")?;
102+
f(&format!{"{}:{}", window.0, window.1})?;
93103
}
94104
Ok(())
95105
}
96106
}
107+
108+
#[cfg(test)]
109+
mod test {
110+
use super::*;
111+
use proto::ToArguments;
112+
113+
fn collect<I: ToArguments>(arguments: I) -> Vec<String> {
114+
let mut output = Vec::<String>::new();
115+
arguments.to_arguments::<_, ()>(&mut |arg| Ok(output.push(arg.to_string()))).unwrap();
116+
output
117+
}
118+
119+
#[test]
120+
fn find_window_format() {
121+
let window: Window = (0, 2).into();
122+
let output = collect(window);
123+
assert_eq!(output, vec!["window", "0:2"]);
124+
}
125+
126+
#[test]
127+
fn find_query_format() {
128+
let mut query = Query::new();
129+
let finished = query.and(Term::Tag("albumartist".into()), "Mac DeMarco")
130+
.and(Term::Tag("album".into()), "Salad Days");
131+
let output = collect(&*finished);
132+
assert_eq!(output, vec!["albumartist", "Mac DeMarco", "album", "Salad Days"]);
133+
}
134+
}

src/status.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,17 @@ impl Encodable for Status {
7070
e.emit_struct_field("song", 8, |e| self.song.encode(e))?;
7171
e.emit_struct_field("nextsong", 9, |e| self.nextsong.encode(e))?;
7272
e.emit_struct_field("time", 10, |e| {
73-
e.emit_option(|e| {
74-
match self.time {
75-
Some(p) => {
76-
e.emit_option_some(|e| {
77-
e.emit_tuple(2, |e| {
78-
e.emit_tuple_arg(0, |e| p.0.num_seconds().encode(e))?;
79-
e.emit_tuple_arg(1, |e| p.1.num_seconds().encode(e))?;
80-
Ok(())
81-
})
73+
e.emit_option(|e| match self.time {
74+
Some(p) => {
75+
e.emit_option_some(|e| {
76+
e.emit_tuple(2, |e| {
77+
e.emit_tuple_arg(0, |e| p.0.num_seconds().encode(e))?;
78+
e.emit_tuple_arg(1, |e| p.1.num_seconds().encode(e))?;
79+
Ok(())
8280
})
83-
}
84-
None => e.emit_option_none(),
85-
};
86-
Ok(())
81+
})
82+
}
83+
None => e.emit_option_none(),
8784
})
8885
})?;
8986
e.emit_struct_field("elapsed", 11, |e| {

tests/search.rs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ extern crate mpd;
22

33
mod helpers;
44
use helpers::connect;
5-
use mpd::{Query, Term};
6-
use mpd::search::Window;
5+
use mpd::Query;
76

87
#[test]
98
fn search() {
@@ -14,17 +13,3 @@ fn search() {
1413
println!("{:?}", songs);
1514
assert!(songs.is_ok());
1615
}
17-
18-
#[test]
19-
fn find_query_format() {
20-
let mut query = Query::new();
21-
let finished = query.and(Term::Tag("albumartist".into()), "Mac DeMarco")
22-
.and(Term::Tag("album".into()), "Salad Days");
23-
assert_eq!(&finished.to_string(), " albumartist \"Mac DeMarco\" album \"Salad Days\"");
24-
}
25-
26-
#[test]
27-
fn find_window_format() {
28-
let window: Window = (0, 2).into();
29-
assert_eq!(&window.to_string(), " window 0:2");
30-
}

0 commit comments

Comments
 (0)