Skip to content

Commit 1fc4976

Browse files
mgretermgreter
committed
Improve native Sass_Value handling
1 parent 7734584 commit 1fc4976

File tree

3 files changed

+150
-94
lines changed

3 files changed

+150
-94
lines changed

‎lib/CSS/Sass.xs

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,18 @@ union Sass_Value* sv_to_sass_value(SV* sv)
9090
if (!SvOK(sv)) return sass_make_null();
9191
// perl reference
9292
if (SvROK(sv)) {
93+
// dereference
94+
sv = SvRV(sv);
9395
// check if it's an error struct
94-
if (SvTYPE(SvRV(sv)) == SVt_PVAV) {
95-
SV** value_svp = av_fetch((AV*)SvRV(sv), 0, false);
96-
SV* value_sv = value_svp ? *value_svp : &PL_sv_undef;
97-
return sass_make_error(SvPV_nolen(value_sv));
96+
if (SvTYPE(sv) == SVt_PVAV) {
97+
bool has_msg = false;
98+
if (av_len((AV*)sv) >= 0) {
99+
SV** value_svp = av_fetch((AV*)sv, 0, false);
100+
has_msg = value_svp && *value_svp && SvOK(*value_svp);
101+
return sass_make_error(has_msg ? SvPV_nolen(*value_svp) : "error");
102+
} else {
103+
return sass_make_error("error");
104+
}
98105
}
99106
// if we have a scalar
100107
} else if (!SvROK(sv)) {
@@ -122,11 +129,16 @@ union Sass_Value* sv_to_sass_value(SV* sv)
122129
// a hash means we have a color
123130
else if (SvTYPE(sv) == SVt_PVHV) {
124131
HV* color = (HV*) sv;
125-
double r = SvNV(*hv_fetchs(color, "r", false));
126-
double g = SvNV(*hv_fetchs(color, "g", false));
127-
double b = SvNV(*hv_fetchs(color, "b", false));
128-
double a = SvNV(*hv_fetchs(color, "a", false));
129-
return sass_make_color(r, g, b, a);
132+
SV* sv_r = *hv_fetchs(color, "r", false);
133+
SV* sv_g = *hv_fetchs(color, "g", false);
134+
SV* sv_b = *hv_fetchs(color, "b", false);
135+
SV* sv_a = *hv_fetchs(color, "a", false);
136+
return sass_make_color(
137+
SvOK(sv_r) ? SvNV(sv_r) : 0,
138+
SvOK(sv_g) ? SvNV(sv_g) : 0,
139+
SvOK(sv_b) ? SvNV(sv_b) : 0,
140+
SvOK(sv_a) ? SvNV(sv_a) : 0
141+
);
130142
}
131143

132144
}
@@ -396,12 +408,13 @@ Sass_Import_List sass_importer(const char* cur_path, Sass_Importer_Entry cb, str
396408
// the expected type is an array
397409
else if (SvTYPE(import_sv) == SVt_PVAV) {
398410
AV* import_av = (AV*) import_sv;
399-
SV** path_sv = av_fetch(import_av, 0, false);
400-
SV** source_sv = av_fetch(import_av, 1, false);
401-
SV** mapjson_sv = av_fetch(import_av, 2, false);
402-
SV** error_msg_sv = av_fetch(import_av, 3, false);
403-
SV** error_line_sv = av_fetch(import_av, 4, false);
404-
SV** error_column_sv = av_fetch(import_av, 5, false);
411+
size_t len = av_len(import_av);
412+
SV** path_sv = len < 0 ? 0 : av_fetch(import_av, 0, false);
413+
SV** source_sv = len < 1 ? 0 : av_fetch(import_av, 1, false);
414+
SV** mapjson_sv = len < 2 ? 0 : av_fetch(import_av, 2, false);
415+
SV** error_msg_sv = len < 3 ? 0 : av_fetch(import_av, 3, false);
416+
SV** error_line_sv = len < 4 ? 0 : av_fetch(import_av, 4, false);
417+
SV** error_column_sv = len < 5 ? 0 : av_fetch(import_av, 5, false);
405418
if (path_sv && SvOK(*path_sv)) path = SvPV_nolen(*path_sv);
406419
if (source_sv && SvOK(*source_sv)) source = SvPV_nolen(*source_sv);
407420
if (mapjson_sv && SvOK(*mapjson_sv)) mapjson = SvPV_nolen(*mapjson_sv);

‎lib/CSS/Sass/Type.pm

Lines changed: 68 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,28 @@ our $VERSION = "v3.2.1";
3131
################################################################################
3232
use CSS::Sass qw(import_sv);
3333
################################################################################
34+
use overload '""' => 'stringify';
35+
use overload 'eq' => 'equals';
36+
use overload '==' => 'equals';
37+
use overload 'ne' => 'nequals';
38+
use overload '!=' => 'nequals';
39+
################################################################################
40+
3441
sub new { import_sv($_[1]) }
42+
sub clone { import_sv($_[0]) }
43+
44+
# default implementations
45+
sub quoted { shift->stringify(@_) }
3546

47+
# generic implementations
48+
sub equals { $_[0]->stringify eq $_[1] ? 1 : 0; }
49+
sub nequals { $_[0]->equals($_[1]) ? 0 : 1; }
3650

3751
################################################################################
3852
package CSS::Sass::Type::Null;
3953
################################################################################
4054
use base 'CSS::Sass::Type';
4155
################################################################################
42-
use overload '""' => 'stringify';
43-
use overload 'eq' => 'equals';
44-
use overload '==' => 'equals';
45-
use overload 'ne' => 'nequals';
46-
use overload '!=' => 'nequals';
47-
################################################################################
4856

4957
sub new {
5058
my ($class) = @_;
@@ -64,15 +72,15 @@ package CSS::Sass::Type::Error;
6472
################################################################################
6573
use base 'CSS::Sass::Type';
6674
################################################################################
67-
use overload '""' => 'stringify';
68-
use overload 'eq' => 'equals';
69-
################################################################################
7075

7176
sub new {
7277
my ($class, @msg) = @_;
7378
bless \\ [ @msg ], $class;
7479
}
7580

81+
# cloning through sass.xs does not work?
82+
# sub clone { bless \\ [ @{${${$_[0]}}} ] }
83+
7684
sub message {
7785
wantarray ? @{${${$_[0]}}} :
7886
join "", @{${${$_[0]}}};
@@ -84,18 +92,11 @@ sub stringify {
8492
: "error";
8593
}
8694

87-
sub equals {
88-
shift->stringify eq $_[0]
89-
}
90-
9195
################################################################################
9296
package CSS::Sass::Type::Boolean;
9397
################################################################################
9498
use base 'CSS::Sass::Type';
9599
################################################################################
96-
use overload '""' => 'stringify';
97-
use overload 'eq' => 'equals';
98-
################################################################################
99100

100101
sub new {
101102
my ($class, $bool) = @_;
@@ -114,53 +115,41 @@ sub stringify {
114115
shift->value ? "true" : "false";
115116
}
116117

117-
sub equals {
118-
shift->stringify eq $_[0]
119-
}
120-
121118
################################################################################
122119
package CSS::Sass::Type::String;
123120
################################################################################
124121
use base 'CSS::Sass::Type';
125122
################################################################################
126-
use CSS::Sass qw(quote unquote);
127-
################################################################################
128-
use overload '""' => 'stringify';
129-
use overload 'eq' => 'equals';
123+
use CSS::Sass qw(quote);
130124
################################################################################
131125

132126
sub new {
133127
my ($class, $string) = @_;
134128
$string = "" unless defined $string;
135-
# we may can unquote the string!
136-
# should we really do this here?
137129
bless \ $string, $class;
138130
}
139131

140132
sub value {
141133
if (scalar(@_) > 1) {
142134
${$_[0]} = defined $_[1] ? $_[1] : "";
143135
}
144-
defined ${$_[0]} ? unquote(${$_[0]}) : "";
136+
defined ${$_[0]} ? ${$_[0]} : "";
145137
}
146138

147139
sub stringify {
148140
$_ = shift->value;
149-
m/^\w*$/ ? $_ : quote($_);
141+
m/^\w*$/ ? $_ : $_;
150142
}
151143

152-
sub equals {
153-
shift->stringify eq $_[0]
144+
sub quoted {
145+
quote($_[0]);
154146
}
155147

156148
################################################################################
157149
package CSS::Sass::Type::Number;
158150
################################################################################
159151
use base 'CSS::Sass::Type';
160152
################################################################################
161-
use overload '""' => 'stringify';
162-
use overload 'eq' => 'equals';
163-
################################################################################
164153

165154
sub new {
166155
my ($class, $number, $unit) = @_;
@@ -187,18 +176,11 @@ sub stringify {
187176
sprintf "%g%s", @{${$_[0]}};
188177
}
189178

190-
sub equals {
191-
shift->stringify eq $_[0]
192-
}
193-
194179
################################################################################
195180
package CSS::Sass::Type::Color;
196181
################################################################################
197182
use base 'CSS::Sass::Type';
198183
################################################################################
199-
use overload '""' => 'stringify';
200-
use overload 'eq' => 'equals';
201-
################################################################################
202184

203185
sub new {
204186
my ($class, $r, $g, $b, $a) = @_;
@@ -234,18 +216,13 @@ sub stringify {
234216
}
235217
}
236218

237-
sub equals {
238-
shift->stringify eq $_[0]
239-
}
240-
241219

242220
################################################################################
243221
package CSS::Sass::Type::Map;
244222
################################################################################
245223
use base 'CSS::Sass::Type';
246224
################################################################################
247-
use overload '""' => 'stringify';
248-
use overload 'eq' => 'equals';
225+
use CSS::Sass qw(quote);
249226
################################################################################
250227

251228
sub new {
@@ -261,11 +238,7 @@ sub keys { CORE::keys %{$_[0]} }
261238
sub values { CORE::values %{$_[0]} }
262239

263240
sub stringify {
264-
join ', ', map { join ": ", $_, $_[0]->{$_} } CORE::keys %{$_[0]};
265-
}
266-
267-
sub equals {
268-
shift->stringify eq $_[0]
241+
join ', ', map { join ": ", quote($_), quote($_[0]->{$_}) } CORE::keys %{$_[0]};
269242
}
270243

271244

@@ -274,8 +247,7 @@ package CSS::Sass::Type::List;
274247
################################################################################
275248
use base 'CSS::Sass::Type';
276249
################################################################################
277-
use overload '""' => 'stringify';
278-
use overload 'eq' => 'equals';
250+
use CSS::Sass qw(SASS_COMMA);
279251
################################################################################
280252

281253
sub new {
@@ -286,8 +258,42 @@ sub new {
286258

287259
sub values { @{$_[0]} }
288260

289-
sub stringify { join ', ', @{$_[0]} }
290-
sub equals { shift->stringify eq $_[0] }
261+
sub listjoint { ', ' }
262+
sub separator { return SASS_COMMA }
263+
264+
sub stringify
265+
{
266+
join $_[0]->listjoint,
267+
map { ref $_ ? $_->quoted : $_ }
268+
@{$_[0]};
269+
}
270+
271+
# ignore different separators
272+
# they have different stringifies
273+
# so we must overload generic method
274+
sub equals {
275+
# compare to native type
276+
unless (ref $_[1]) {
277+
return $_[0]->stringify eq $_[1]
278+
}
279+
# compare to another list (compore arrays)
280+
elsif ($_[1]->isa('CSS::Sass::Type::List')) {
281+
# both arrays must have same length
282+
return 0 if $#{$_[0]} != $#{$_[1]};
283+
# all items must be equal to the other items
284+
for (my ($i, $L) = (0, scalar(@{$_[0]})); $i < $L; $i++)
285+
{ return 0 if $_[0]->[$i] ne $_[1]->[$i]; }
286+
# no diffs
287+
return 1;
288+
}
289+
# other value
290+
else
291+
{
292+
return 0;
293+
# is always false
294+
# $_[0] !== $_[1];
295+
}
296+
}
291297

292298
################################################################################
293299
package CSS::Sass::Type::List::Comma;
@@ -298,7 +304,7 @@ use CSS::Sass qw(SASS_COMMA);
298304
################################################################################
299305
sub new { shift->SUPER::new(@_) }
300306
sub separator { return SASS_COMMA }
301-
sub stringify { join ', ', @{$_[0]} }
307+
sub listjoint { ', ' }
302308

303309
################################################################################
304310
package CSS::Sass::Type::List::Space;
@@ -309,7 +315,7 @@ use CSS::Sass qw(SASS_SPACE);
309315
################################################################################
310316
sub new { shift->SUPER::new(@_) }
311317
sub separator { return SASS_SPACE }
312-
sub stringify { join ' ', @{$_[0]} }
318+
sub listjoint { ' ' }
313319

314320
################################################################################
315321
package CSS::Sass::Type;
@@ -385,19 +391,19 @@ It only implements a generic constructor, which accepts native perl data types
385391
=head2 CSS::Sass::Type::Map
386392
387393
my $map = CSS::Sass::Type::Map->new(key => 'value');
388-
my $string = "$map"; # eq 'key: value'
389-
my $value = $map->{'key'}; # eq 'value'
394+
my $string = "$map"; # eq 'key: "value"'
395+
my $value = $map->{'key'}; # eq '"value"'
390396
391397
=head2 CSS::Sass::Type::List::Comma
392398
393399
my $list = CSS::Sass::Type::List::Comma->new('foo', 'bar');
394-
my $string = "$list"; # eq 'foo, bar'
400+
my $string = "$list"; # eq '"foo", "bar"'
395401
my $value = $list->[0]; # eq 'foo'
396402
397403
=head2 CSS::Sass::Type::List::Space
398404
399405
my $list = CSS::Sass::Type::List::Space->new('foo', 'bar');
400-
my $string = "$list"; # eq 'foo bar'
406+
my $string = "$list"; # eq '"foo" "bar"'
401407
my $value = $list->[-1]; # eq 'bar'
402408
403409
=head1 SEE ALSO
@@ -406,7 +412,7 @@ L<CSS::Sass>
406412
407413
=head1 AUTHOR
408414
409-
David Caldwell E<lt>[email protected]E<gt>
415+
David Caldwell E<lt>[email protected]E<gt>
410416
Marcel Greter E<lt>[email protected]E<gt>
411417
412418
=head1 LICENSE

0 commit comments

Comments
 (0)