Skip to content

Commit 1cac773

Browse files
committed
Improve error formatting code.
Make `Debug` formatting of `rsass::Error` look like the `Display` output, but without the "Error: " prefix. This makes the error display correctly if returned from a `main` function. This also puts the "Error: " prefix in one single place, rather than repeating it in error messages all over the codebase.
1 parent 5416455 commit 1cac773

File tree

10 files changed

+38
-38
lines changed

10 files changed

+38
-38
lines changed

Diff for: CHANGELOG.md

+6
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ project adheres to
3131
* The sass `Value::Variable` and `Item::VariableDeclaration` variants
3232
now holds a `Name` rather than just a `String` for the variable name.
3333
Also, both now holds a `SourcePos`.
34+
* `Error::error` now takes an `Into<String>` argument (PR #151).
3435

3536
### Improvements
3637

@@ -44,8 +45,13 @@ project adheres to
4445
* The css `var(...)` function is now parsed as a proper function, and not
4546
as a special string (PR #147).
4647
* The null value can be quoted as an empty string (PR #147).
48+
* Make `Debug` formatting of `rsass::Error` look like the `Display` output,
49+
but without the "Error: " prefix. This makes the error display correctly
50+
if returned from a main function. This also removed the "Error: " prefix
51+
from a lot of message strings (PR #151).
4752
* In error message, don't show ellipses for consecutive lines (PR #147).
4853
* Somtimes a trailing comma in argument lists is preserved (PR #147).
54+
* Simplified `main` of the command-line by returning a `Result` (PR #151).
4955
* Update sass-spec test suite to 2022-06-21.
5056
* Some cleanups.
5157

Diff for: src/error.rs

+12-8
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use std::convert::From;
66
use std::{fmt, io};
77

88
/// Most functions in rsass that returns a Result uses this Error type.
9-
#[derive(Debug)]
109
pub enum Error {
1110
/// An IO error encoundered on a specific path
1211
Input(String, io::Error),
@@ -40,32 +39,37 @@ impl std::error::Error for Error {}
4039

4140
impl Error {
4241
/// A generic error message.
43-
pub fn error<T: AsRef<str>>(msg: T) -> Self {
44-
Error::S(format!("Error: {}", msg.as_ref()))
42+
pub fn error<T: Into<String>>(msg: T) -> Self {
43+
Error::S(msg.into())
4544
}
4645
}
4746

4847
impl fmt::Display for Error {
4948
fn fmt(&self, out: &mut fmt::Formatter) -> fmt::Result {
49+
write!(out, "Error: {:?}", self)
50+
}
51+
}
52+
53+
impl fmt::Debug for Error {
54+
fn fmt(&self, out: &mut fmt::Formatter<'_>) -> fmt::Result {
5055
match *self {
5156
Error::S(ref s) => write!(out, "{}", s),
5257
Error::Input(ref p, ref e) => {
5358
write!(out, "Failed to read {:?}: {}", p, e)
5459
}
5560
Error::BadArgument(ref name, ref problem) => {
56-
write!(out, "Error: ${}: {}", name, problem)
61+
write!(out, "${}: {}", name, problem)
5762
}
5863
Error::ParseError(ref err) => err.fmt(out),
5964
Error::ImportLoop(ref pos, ref oldpos) => {
60-
writeln!(out, "Error: This file is already being loaded.")?;
65+
writeln!(out, "This file is already being loaded.")?;
6166
pos.show_detail(out, '^', " new load")?;
6267
writeln!(out)?;
6368
oldpos.show_detail(out, '=', " original load")?;
6469
pos.show_files(out)
6570
}
6671
Error::BadCall(ref msg, ref callpos, ref declpos) => {
67-
msg.fmt(out)?;
68-
writeln!(out)?;
72+
writeln!(out, "{}", msg)?;
6973
if let Some(declpos) = declpos {
7074
if callpos.same_file_as(declpos) {
7175
show_in_file(
@@ -86,7 +90,7 @@ impl fmt::Display for Error {
8690
}
8791
}
8892
Error::Invalid(ref what, ref pos) => {
89-
writeln!(out, "Error: {}", what)?;
93+
writeln!(out, "{}", what)?;
9094
pos.show(out)
9195
}
9296
Error::BadRange(ref err) => err.fmt(out),

Diff for: src/main.rs

+2-9
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,9 @@ use rsass::{
55
};
66
use std::io::{stdout, Write};
77
use std::path::PathBuf;
8-
use std::process::exit;
98

10-
fn main() {
11-
match Args::parse().run() {
12-
Ok(()) => (),
13-
Err(err) => {
14-
eprintln!("{}", err);
15-
exit(1);
16-
}
17-
}
9+
fn main() -> Result<(), Error> {
10+
Args::parse().run()
1811
}
1912

2013
#[derive(Parser)]

Diff for: src/output/transform.rs

+5-7
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ fn handle_item(
7474
{
7575
if !with.is_empty() {
7676
return Err(Error::BadCall(
77-
"Error: Built-in modules can\'t be configured."
78-
.into(),
77+
"Built-in modules can\'t be configured.".into(),
7978
pos.clone(),
8079
None,
8180
));
@@ -117,7 +116,7 @@ fn handle_item(
117116
})?
118117
} else {
119118
return Err(Error::BadCall(
120-
"Error: Can't find stylesheet to import.".into(),
119+
"Can't find stylesheet to import.".into(),
121120
pos.clone(),
122121
None,
123122
));
@@ -130,8 +129,7 @@ fn handle_item(
130129
{
131130
if !with.is_empty() {
132131
return Err(Error::BadCall(
133-
"Error: Built-in modules can\'t be configured."
134-
.into(),
132+
"Built-in modules can\'t be configured.".into(),
135133
pos.clone(),
136134
None,
137135
));
@@ -221,7 +219,7 @@ fn handle_item(
221219
|| x.is_css_url())
222220
{
223221
return Err(Error::BadCall(
224-
"Error: Can't find stylesheet to import.".into(),
222+
"Can't find stylesheet to import.".into(),
225223
pos.clone(),
226224
None,
227225
));
@@ -360,7 +358,7 @@ fn handle_item(
360358
})?;
361359
} else {
362360
return Err(Error::BadCall(
363-
"Error: Undefined mixin.".into(),
361+
"Undefined mixin.".into(),
364362
pos.clone(),
365363
None,
366364
));

Diff for: src/sass/formal_args.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -159,25 +159,25 @@ impl fmt::Display for ArgsError {
159159
match self {
160160
ArgsError::TooManyPos(n, m) => write!(
161161
out,
162-
"Error: Only {} positional argument{} allowed, but {} {} passed.",
162+
"Only {} positional argument{} allowed, but {} {} passed.",
163163
n,
164164
if *n != 1 { "s" } else { "" },
165165
m,
166166
if *m != 1 { "were" } else { "was" },
167167
),
168168
ArgsError::TooMany(n, m) => write!(
169169
out,
170-
"Error: Only {} argument{} allowed, but {} {} passed.",
170+
"Only {} argument{} allowed, but {} {} passed.",
171171
n,
172172
if *n != 1 { "s" } else { "" },
173173
m,
174174
if *m != 1 { "were" } else { "was" },
175175
),
176176
ArgsError::Missing(name) => {
177-
write!(out, "Error: Missing argument ${}.", name)
177+
write!(out, "Missing argument ${}.", name)
178178
}
179179
ArgsError::Unexpected(name) => {
180-
write!(out, "Error: No argument named ${}.", name)
180+
write!(out, "No argument named ${}.", name)
181181
}
182182
ArgsError::Eval(e) => e.fmt(out),
183183
}

Diff for: src/sass/functions/color/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ fn bad_arg(err: ArgsError, name: &Name, args: &FormalArgs) -> Error {
183183

184184
fn not_in_module(nm: &Name, col: &Value, an: &Name, av: &Value) -> Error {
185185
Error::S(format!(
186-
"Error: The function {0}() isn\'t in the sass:color module.\n\
186+
"The function {0}() isn\'t in the sass:color module.\n\
187187
\nRecommendation: color.adjust({1}, ${2}: {3})\n\
188188
\nMore info: https://sass-lang.com/documentation/functions/color#{0}",
189189
nm,

Diff for: src/sass/functions/meta.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ pub fn create_module() -> Scope {
111111
} else if let Some(f) = get_function(s, module, name.value())? {
112112
Ok(Value::Function(name.value().into(), Some(f)))
113113
} else {
114-
Err(Error::S(format!("Error: Function not found: {}", name)))
114+
Err(Error::S(format!("Function not found: {}", name)))
115115
}
116116
}
117117
);

Diff for: src/sass/mixin.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl MixinDecl {
7373
.map_err(|e| e.decl_called(call_pos2, pos))?;
7474
let call_pos2 = call_pos.clone();
7575
let url = get_string(&argscope, name!(url)).map_err(|e| {
76-
Error::BadCall(e.to_string(), call_pos2, None)
76+
Error::BadCall(format!("{:?}", e), call_pos2, None)
7777
})?;
7878
let call_pos2 = call_pos.clone();
7979
let with = get_opt_map(&argscope, name!(with))
@@ -85,7 +85,7 @@ impl MixinDecl {
8585
} else {
8686
return Err(Error::BadCall(
8787
format!(
88-
"Error: Built-in module {} can't be configured.",
88+
"Built-in module {} can't be configured.",
8989
url.value()
9090
),
9191
call_pos.clone(),
@@ -98,7 +98,7 @@ impl MixinDecl {
9898
.find_file_use(url.value(), call_pos.clone())?
9999
.ok_or_else(|| {
100100
Error::BadCall(
101-
"Error: Can't find stylesheet to import.".into(),
101+
"Can't find stylesheet to import.".into(),
102102
call_pos2,
103103
None,
104104
)
@@ -160,10 +160,10 @@ pub(crate) fn get_opt_map(
160160
.try_into()
161161
.map_err(|e| match e {
162162
ValueToMapError::Root(err) => {
163-
format!("Error: ${}: {}", name, err)
163+
format!("${}: {}", name, err)
164164
}
165165
ValueToMapError::Key(err) => {
166-
format!("Error: ${} key: {}", name, err)
166+
format!("${} key: {}", name, err)
167167
}
168168
})
169169
.map(Some),

Diff for: src/sass/value.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,11 @@ impl Value {
149149
Some(decl),
150150
),
151151
Error::Invalid(Invalid::AtError(msg), _) => {
152-
let msg = format!("Error: {}", msg);
153152
Error::BadCall(msg, pos.clone(), None)
154153
}
155154
e => {
156155
let pos = pos.clone().opt_in_calc();
157-
Error::BadCall(e.to_string(), pos, None)
156+
Error::BadCall(format!("{:?}", e), pos, None)
158157
}
159158
});
160159
}

Diff for: tests/spec/main.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//! Tests auto-converted from "sass-spec/spec"
2-
//! version 50ee810b, 2022-06-21 17:35:49 -0700.
2+
//! version dcff661e, 2022-07-06 13:41:49 -0700.
33
//! See <https://github.com/sass/sass-spec> for source material.\n
44
//! The following tests are excluded from conversion:
55
//! ["core_functions/selector/extend", "core_functions/selector/is_superselector", "core_functions/selector/unify", "directives/extend", "libsass-todo-issues/issue_221262.hrx", "libsass-todo-issues/issue_221292.hrx", "libsass/unicode-bom/utf-16-big", "libsass/unicode-bom/utf-16-little", "non_conformant/scss/huge.hrx", "non_conformant/scss/multiline-var.hrx"]

0 commit comments

Comments
 (0)