Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shared interfaces for various AST nodes #1445

Merged
merged 3 commits into from Aug 23, 2021
Merged

Add shared interfaces for various AST nodes #1445

merged 3 commits into from Aug 23, 2021

Conversation

jathak
Copy link
Member

@jathak jathak commented Aug 20, 2021

Fixes #1401 and #1414.

Adds Dependency, SassDeclaration, and SassReference interfaces,
which expose some getters that multiple AST nodes have in common with a
single type.

These also add getters for common subspans (URL, name, and namespace) to
the interfaces.

To allow for FunctionExpression to have a common interface with the
other two reference nodes, its old name getter has been renamed to
interpolatedName, and the new name getter now is either just a
string, or null if the name is interpolated.

Fixes #1401 and #1414.

Adds `Dependency`, `SassDeclaration`, and `SassReference` interfaces,
which expose some getters that multiple AST nodes have in common with a
single type.

These also add getters for common subspans (URL, name, and namespace) to
the interfaces.

To allow for `FunctionExpression` to have a common interface with the
other two reference nodes, its old `name` getter has been renamed to
`interpolatedName`, and the new `name` getter now is either just a
string, or null if the name is interpolated.
@jathak jathak requested a review from nex3 August 20, 2021 20:20
lib/src/ast/sass/interface/declaration.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/interface/declaration.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/interface/dependency.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/interface/reference.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/argument.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/expression/function.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/expression/variable.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/interface/reference.dart Outdated Show resolved Hide resolved
lib/src/ast/sass/statement/forward_rule.dart Outdated Show resolved Hide resolved
pubspec.yaml Show resolved Hide resolved
@jathak jathak requested a review from nex3 August 23, 2021 17:13
? interpolatedName.asPlain?.replaceAll('_', '-')
: interpolatedName.asPlain;
/// If this function is a plain CSS function, use [originalName] instead.
final String name;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make this a getter, since a single String.replaceAll() isn't very expensive.

Comment on lines 81 to 83
var quote = skipRule.text[0];
var end = skipRule.text.indexOf(quote, 1);
return skipRule.subspan(0, end + 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be worth making a Span utility as well.

Comment on lines 47 to 49
if (namespace != null) {
startSpan = startSpan.withoutInitialIdentifier().subspan(1);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding SpanExtensions.withoutNamespace().

}
}
int escapeCharacter() =>
consumeEscapedCharacter(scanner);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: reformat

return scanner.readChar();
}
}

extension SpanExtensions on FileSpan {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is big enough now that it should maybe go into its own file.

@jathak jathak merged commit d419df7 into main Aug 23, 2021
@jathak jathak deleted the helper-interfaces branch August 23, 2021 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more fine-grained spans to some AST nodes
2 participants